book module now without clearfix & better markup + accessibility love

Thanks for rolling this patch. I was just noticing this problem this week and didn't remember if we had an open issue.

IMO this is actually critical, the links in head right now make little to no sense since there is no context. That being said functionality isn't broken, but ux is severely diminished.

Looks like the book module simpletests just need to be updated.

This is a WCAG A issue:

2.4.4 Link Purpose (In Context): The purpose of each link can be determined from the link text alone or from the link text together with its programmatically determined link context,except where the purpose of the link would be ambiguous to users in general.(Level A)

http://www.w3.org/TR/UNDERSTANDING-WCAG20/navigation-mechanisms-refs.html

The title attributes on the anchors will not be available to most screen-reader users, and some screen-readers (e.g. JAWS) will not read the characters in the link text like ‹ . Can we somehow add this information to the anchor text?

Also, when using characters like ‹ we should be using html entities like »

Thanks, looking good. Final comments.

1. These links should be proceeded by a heading like the pagination links on the default front page.
2. Do we need to get rid of the title attribute, or should we be doing title + invisible span?
3. I'm thinking the text in the invisible span is a bit verbose.

Good catch. However, while worth to fix, this bug is not really major.

Tagging with markup to make sure that the changes are acceptable to the markup maintainers.

Some minor comments:

• book.css should have 2 spaces preceding the line 'list-style: none;' and adds an unneeded empty line at the bottom.
• book.test has the number 2 in the string "Previous page link found2." Is this part of your magic to pass the test bot?

For the rest the patch looks fine, it makes the HTML of the book navigation semantically meaningful.

Looks fine to me now.

Tried applying this patch but got:

deimos:drupal-head dries$ git apply < ../f.p 
error: No changes

Patch might be in the wrong format?

Tried applying this patch but got:

deimos:drupal-head dries$ git apply < ../f.p 
error: No changes

Patch might be in the wrong format?

I'm not going to touch the status right now but with regards to workflow etc this will need some CSS for Bartik, maybe even D8 core(?), looking at the screenshot in #6 we can see this borks out Garland so no way we can backport without CSS fixes for both themes in D7 (Bartik & Garland) - either we do that here or in new issues, and possible block a point release until such time that CSS is ready?

There are some white space issues in this patch.

Wondering if the titles are too verbose - things like "Go to previous page", could that be better said as just "Previous page", thinking about how screen readers will synthesize this, something along the lines of:

link, [link-title], Previous page

To me that makes grammatical sense, however this reads/sounds a little odd:

link, [link-title], Go to previous page

Any strong reason for removing the wrapper? In HTML5 we could use a nav or footer wrapper for these.

+1 for 'Previous page' It's a link, I can infer that I am going somewhere :)

I think just leave the wrapper and go for the main issue - the accessibility improvement, we have an issue open for HTML5ifying this template: #1189828: Convert book-navigation.tpl.php to HTML5

Here is an updated patch that removes the whitespaces errors upon applying it.

This is indeed an improvement over the current markup.
However i noticed there is still a layout problem where the book navigation borders are shown twice. (see attached screenshot)
I tested this in both Bartik & Seven.

This is caused by the fact that both the div and the ul in book-navigation.tpl.php have the CSS class page-links.
I'm sure that's not the intended behaviour?

Just to clarify, I am not currently working on this issue, just moving it to the correct component.

This is certainly part of Book.module, but it is a Markup issue. In order for Markup / CSS / JS folk to find issues easily we are moving issues into most appropriate component.

Sometimes it's a bit unclear about where something should go, but the core issue here is about markup, not about how Book behaves.

hey
just wanna inform you that were working on cleaning up the css & follow the BAT rules for the filenaming over here : http://drupal.org/node/1216970
so we should keep an eye out for eachother so we dont end up crashing ;)

I marked this #1189828: Convert book-navigation.tpl.php to HTML5 a duplicate of this issue. There's no point in looking at this twice, as we ultimately want the same results here.

We can definitely work here... There's more done here anyway. The other issue had no patch, so I closed it.

As far as D7 goes, @webchick will never allow this to be backported. Sorry about that! I should have stated that when I removed the tag.

Actually, I will ask @webchick to chime in on that rather than speaking for her. I am just 99.9% sure it's a no-go for D7 based on previous experience. ;)

Here's a go at it. It's my first patch, so if there are errors, I apologize. Working from the previous patch, I replaced the wrapper div with a more semantic nav element and added the rel=”next” and rel=”prev” links to the first and last (http://dev.w3.org/html5/spec/links.html#sequential-link-types). Seems that rel="up" has been deprecated in the HTML5 spec. I didn't add any CSS changes as Jacine asked me just to stab at the markup first.

/me hires Jacine as official spokesperson!

That's correct. .tpl.php files are locked down for D7, and have been since the end of polish phase in December 2010.

Here is another take at it. Added the clearfix class back on the ul in the markup – I was thinking it would be ideal to do without it, but the current CSS would depend on it with the floats. Also just set the ul to list-style:none in the CSS and added the rel tags to the test.

okay looks like i kinda tried to hijack this issue over at the css cleanup
So i wanted som thoughts on changing the markup for the navigation to use li so we end up with the same markup & classes as the pager.

book-navigation.tpl:

<?php if ($tree || $has_links): ?>
  <div class="book-navigation">
    <?php print $tree; ?>

    <?php if ($has_links): ?>
    <ul class="pager clearfix">
      <h2 class="element-invisible"><?php print t('Book Navigation'); ?></h2>    
      <?php if ($prev_url): ?>
        <li class="page-previous"><a href="<?php print $prev_url; ?>" rel="next" title="<?php print t('Go to previous page'); ?>"><?php print t('‹ ') . $prev_title; ?></a></li>
      <?php endif; ?>
      <?php if ($parent_url): ?>
        <li class="page-up"><a href="<?php print $parent_url; ?>" title="<?php print t('Go to parent page'); ?>"><?php print t('up'); ?></a></li>
      <?php endif; ?>
      <?php if ($next_url): ?>
        <li class="page-next"><a href="<?php print $next_url; ?>" rel="prev" title="<?php print t('Go to next page'); ?>"><?php print $next_title . t(' ›'); ?></a></li>
      <?php endif; ?>
    </ul>
    <?php endif; ?>

  </div>
<?php endif; ?>

changes in the book.theme.css.

.book-navigation .pager {
  border-bottom: 1px solid #888;
  border-top: 1px solid #888;
  list-style-type: none;
  padding: 0.5em;
  text-align: center;
}

im gonna roll a patch later today - but thought it was worth a discussion ;)

I have marked a #1216970: Clean up the CSS for Book module duplicate and rolled the patch together with the one here with an added nav love

Ok, so the markup is good and the tests pass! Yay!

The CSS tests well (even in IE7) but there are code style some issues:

+++ b/core/modules/book/book.theme.cssundefined
@@ -0,0 +1,42 @@
+.book-navigation .menu {
+  border-top: 1px solid #D6D6D6;
+  padding: 1em 0 0 3em; /* LTR */
+}
+
+

There shouldn't be any empty lines after this.

+++ b/core/modules/book/book.theme.cssundefined
@@ -0,0 +1,42 @@
+  margin:0;

There needs to be a space after the colon, before the value.

+++ b/core/modules/book/book.theme.cssundefined
@@ -0,0 +1,42 @@
+
+.pager li{

The empty line above the rule needs to go and there should be a space after li and before the opening bracket.

+++ b/core/modules/book/book.theme.cssundefined
@@ -0,0 +1,42 @@
+  float:left;

There needs to be a space after the colon, before the value.

Now... this .pager style stuff. What is the deal? It seems like you're trying to make this a globalized style, because you prefixed these selectors with the .pager class, but what happens when you've got both the book CSS and the system.theme.css CSS on the same page, and you've customized the pager styles? A conflict on .pager li is gonna happen for sure...

Looking at this deeper, the class names on the list items don't use the same naming conventions. If we were trying to do that, then it'd be: pager-next, pager-previous and pager-up instead of page-next, page-previous and page-up.

I don't know. Honestly, I don't think the pager class here is a good idea. This requires all all sorts of floating and different presentation, whereas the regular pager could just use display: inline, and I am worried about this getting messed up and being annoying when a theme does heavy styling for the regular pagers. At the end of the day the only thing they actually share is list-style-type: none, so it doesn't really seem worth it.

What's the benefit do you see from doing this?

Since the book pager is functionally different from a normal Drupal pager, I agree that a themer will end up with too much CSS overwrite issues if we use the .pager class here.

Other than the issues mentioned by Jacine, this patch looks good to me.

alright i hear ya - i was a little tricker happy there

lets try this again...

Even that the book module is now ready to go - i kinda I played around further with the book navigation module, now that we opened up the box so i changed a couple of things to make it even better.
The < & > is wrapped in a b tags, so we can do stuff add icons whatever to the arrows, or remove em directly with a display none etc. - This gives us way more flexibility, than we got today

Changed the css a little so we can remove the .clearfix class *yeah*

the css is marked with the fixes for ie7 + commented quite heavy & the book.test needs an update.

I have added a couple of screenshots for different browsers & RTL lang as well (only the ie's ;)

changes the title

can anyone tell me why the › is inside a t( ) ?
is there any reason for that other than old legacy it was then easy to change in the t function instead of using the tpl file ?

@mortendk
It allows you to translate › to ‹ for rtl languages perhaps?

+++ b/core/modules/book/book.theme-rtl.cssundefined
@@ -0,0 +1,22 @@
+.book-navigation .page-up {
+  *float: right; /*IE7*/ ¶
+}

+++ b/core/modules/book/book.theme.cssundefined
@@ -0,0 +1,47 @@
+  *zoom: 1;  /* IE7 */
+  *display: inline;	/* IE7 */
+  *float:left; /* IE7 we need this so we dont end up having a obscure breakline when floating right on page-next */
...
+  *display: inline;	/* IE7 */ ¶
...
+  *zoom: 1;/* IE7 */ ¶

move to ie.css ??

20 days to next Drupal core point release.

guess we should have an ie7.css file - but isnt that a thing for the whole of core, so we can collect the garbage one place ?
or should we place them as module.ie7.css thats offcourse starting a whole new amount of css files to be loaded
the reason the comments is so we can find em again in x months when its decided to kill ie7 support (as im expecting ;))

I clean up some codes.

removed b tag and move into t(), although this is a title but I think some guy want to change it to other format :)

any reason when not use the standard .clearfix ??

+.page-links li {
+  display: inline-block; 
+  list-style-type: none; 
+  letter-spacing: normal; /*fix the letter spacing -4px from the parent*/
+  *zoom: 1;  /* IE7 */
+  *display: inline;	/* IE7 */
+  *float:left; /* IE7 we need this so we dont end up having a obscure breakline when floating right on page-next */
+}

when we use display: inline-block, space between left & right is different.

+ letter-spacing: -4px;
This is no sense to me, I don't really get it :(

Thats not cleaning up - thats removing the functionality.

Youre missing the point with the b tags, and the flexibility in the layout it gives us,as an exampla i added a little bit of margin on the links to make it easier on the eyes (and not with a space as its done today) - Without the b (could as well be spans etc) theres no way to control the < + > the spacing, padding margin borders background etc on these 2 "arrows" - it needs a wrapper.
The reason for the b is that is small and easy to read in the css now a designer can by changing the css do what he wants with the pager "arrows", a good side effect is that they have a little bit of heigher weight (bold) than normally which make em naturally stand out.

The whole idea here is that we dont need the clearfix. Clearfix is a nessesary evil. A trick where we try to force a block to wrap around blocks inside it. Spreading it around the drupal core by hardcoding it as a class on every element, is making our codebase it less flexible for the frontend developers (core has now about 100 places where the clearfix is thrown in) - more about the evilness of clearfix : http://thinkvitamin.com/design/everything-you-know-about-clearfix-is-wrong/
We cant align elements with clearfix up next to eachother. When theres ways to get around this by using clean css, we should indeed use it.
The hardcoded clearfix, is a pain to owerwrite, and is frankly a little like putting everything in tabels cells "because that works"

By using the inline-block we can set a width of an element, but its still floating naturally, which a block elements dosnt do unless you float it left & we can now position the books navigation as we want to & dont end up in wrapping that in another div just to remove the clearfix.

The letter spacing at -4 px is to remove the spacing the display: inline-block gives around every li element (we can use font-size:0 instad) its set on the ul (in this case the .page-links) its then cancelled on the li elements , and it all keeps its natural behaviour.

Now the links align up as we would have with a display:block float:left, but we dont have to wrap it in an clearfix that remove our posibility to align the navigation with whatever we might wants

voila everybody wins :)

The reason for the ie7 fixes are placed like that in the code is so we can find em again quickly by searching for /* IE7 */

btw "when we use display: inline-block, space between left & right is different." you have a screenshot + browser so i can check that ?

I have just tested the code on almost all the browsers I have, I dont have a *nix though :(
but heres the screenshots for the test on verious browsers from ie7 to opera + in stark & Bartik.

Theres a minor problem with the supixel rendering using % so theres 1% missing in the calculation (and not the 8% that are missing from core in d7) i think thats actually acceptable & is very easy fixed by a theme it it wants absolute widths, then its easy overwritten.

+++ b/core/modules/book/book.theme.cssundefined
@@ -0,0 +1,47 @@
+  letter-spacing: -4px; /* inline block 4px letter spacing, they are removed here but added again to the li */

why -4px and not -3px or other vaule. when happen if we remove it.

+++ b/core/modules/book/book-navigation.tpl.phpundefined
@@ -30,22 +29,27 @@
+          <a href="<?php print $prev_url; ?>" rel="prev" title="<?php print t('Go to previous page'); ?>"><b><?php print t('‹'); ?></b><?php print $prev_title; ?></a>
...
+          <a href="<?php print $next_url; ?>" rel="next" title="<?php print t('Go to next page'); ?>"><?php print  $next_title;?><b><?php print t('›'); ?></b></a>

wrap inside t function is better ?

and trailing space problem in the patch. (also double space: print $next_title)

19 days to next Drupal core point release.

The reason for -4px is thats is the value added by default by the block-inline which is a PITA , but thats just how it is (and make sense when its inside a p etc.)
In this case though after som playing around -3px was actually working better with the subpixel renedering, cause by the % - So the centering of the up link, after an adjustment of the width to 9.8% was almost perfect, with the exception of Opera 11.52 which IMHO is ok.

stupid whitespace

+++ b/core/modules/book/book.theme.cssundefined
@@ -0,0 +1,47 @@
+  letter-spacing: -3px; /* -3px instead of -4 to push it the last pixel to better centeren of the up link */

This sentence is kinda weird although I understand it

+++ b/core/modules/book/book.theme.cssundefined
@@ -0,0 +1,47 @@
+  width:9.8%; /* subpixel fun */

Lol I'm not sure this accepted as a comment

So if this approach gets accepted it only need "better" docs in the css file and than this is done.

19 days to next Drupal core point release.

Needs review :)

now better & more documentation

Welcome to core development, Morton. Trial by fire is the only way!

Is there a particular reason we are naming the new file book.theme.css and not just book.css? CSS files always have to do with themeing as they are about presentation. Seems wordy for no benefit.

moshe you should read: http://drupal.org/node/1089868

+++ b/core/modules/book/book-navigation.tpl.phpundefined
@@ -30,22 +29,27 @@
+          <a href="<?php print $prev_url; ?>" rel="prev" title="<?php print t('Go to previous page'); ?>"><b><?php print t('‹'); ?></b><?php print $prev_title; ?></a>

I would recommend wrap all links text inside a t function. but if you are don't like it, this is a better one I think, Instead hardcoding the B tag: print t('<b>‹</b>');

+++ b/core/modules/book/book.theme.cssundefined
@@ -0,0 +1,48 @@
+   padding: 1em 0 0 3em; /* LTR */

other UL has 2.5em, may keep it same to other ??

#94 try to remove all padding value here:

+++ b/core/modules/book/book.theme.cssundefined
@@ -0,0 +1,35 @@
+ .book-navigation {
+  border-bottom: 1px solid #D6D6D6;
+  border-top: 1px solid #D6D6D6;
+ }
+.book-navigation .page-links {
+  border-top: 1px solid #D6D6D6;
+  margin: 0;
+  padding: 0.5em 0;
+  overflow: auto;
+}

letter-space is hacking css bug, but by remove it the page is still working well. I seen no reason to keep it

18 days to next Drupal core point release.

@droplet:
Markup in a translations ... That is not the way to do it - then we suddenly have markup that changes based on a translation, talk about debuggin hell ;)
so that is a what i consider a no-go.
Markup lives in eater the theme layer or in a theme function -else were ending with total confusion. Unless i really misunderstood something ??

The letterspacing "trick" is not a css bug - it works as designed - were just using it in another than the css fathers intended (and until the flexi box comes)

At this point there isnt any guidelines of margin & paddings in core . seems like "someone" have set some way back in time.
I would suggest that we dont take that task here but opens up a seperate bug about generalising all margin & paddings in drupal.

but there might be a point in no having lines etc, even that its kinda seen as a drupal default, since the dawn of book module

@moshe yeah i finally made it in ! ... fire ill give you fire ;)
its all about the BAT filings, so the css is gonna be seprated n stuff ;)

-1 for RBTC (reason: #106)

unless somebody tell me there SHOULD be another issue for clean up book module, otherwise I think it worth to clean up the padding/margins here.

sorry about it. willing to see other's opinion :)

I don't want to steal your patch here @mortendk, but there were so many small remarks that creating a new patch was easier than listing them all.

This was done in the new patch:
- removed an extra white line at the end of book.theme.css
- removed an extra white space in front of every line in book.theme.css
- added some missing spaces between properties and their values
- alphabetized all properties since this wasn't always the case
- agree with @droplet to change the left padding on the menu to 2.5em since that seems to be the standard on all other li's in Drupal core
- updated the comments a bit more
- tested if we can remove the letter-spacing mentioned by @droplet, but doing so breaks the layout so this 'hack' needs to stay in unfortunately
- totally agree with mortendk that the markup should be left outside of the translation system.

One remark that I did not put in the patch, is that maybe something like ' is better than using the 'b' tag in the links in book-navigation.tpl.php?

yeah some crap happend with the last patch, dont patch in an airport with screaming kids - i saw it after takeoff - fixed it and was about to submit it, when i saw you patch ;) ooh well that can teach me not to cross borders on a friday night.

yup the letter spacing is the only way to remove that spacing from the inline-block. But removes the clearfix which is an actual hack & in this situation it dosnt matter because of the ul li combination where its the only place its added - and please stop call it a hack when it isnt ;)

the 2.5 em is a good catch think the 3em was something that have been floating around since the dawn of the book module (im stille wondering where those 2.5ems and others are coming from and why they were decided upon)

So what you mean with "One remark that I did not put in the patch, is that maybe something like ' is better than using the 'b' tag in the links in book-navigation.tpl.php?" - you wanna look at something like a span with a loong class name etc in or ?

Yeah, I was thinking a bit more about it overnight, and we should leave the 'b' tag as is :)

Curious where $prev_title and $next_title are defined and what their contents are.

+          <a href="<?php print $parent_url; ?>" title="<?php print t('Go to parent page'); ?>"><?php print t('up'); ?></a>

I don't know that 'up' really says very much about the purpose of this link.

@jyvs,
last patches changed .page-up width to 9.8%, so removes letter space doesn't apply anymore. it should be leave there now.

needs move it up ?

vertical-align: top;

and why not totally remove the paddings (use default/ pre-defined /drupal reset style):

OLD:

.book-navigation .menu {
  border-top: 1px solid #D6D6D6;
  padding: 1em 0 0 2.5em; /* LTR */
}
.book-navigation .page-links {
  border-bottom: 1px solid #D6D6D6;
  border-top: 1px solid #D6D6D6;
  letter-spacing: -3px; /* removes unwanted white space added by inline-block */
  margin: 0;
  overflow: auto;
  padding: 0.5em 0;
}

NEW:

.book-navigation {
  border-bottom: 1px solid #D6D6D6;
  border-top: 1px solid #D6D6D6;
}
.book-navigation .page-links {
  border-top: 1px solid #D6D6D6;
  letter-spacing: -3px; /* removes unwanted white space added by inline-block */
  overflow: auto;
  margin: 0;
  padding: 0.5em 0;
}

I didn not test it but do we have to duplicate border-top?

.book-navigation {
  border-bottom: 1px solid #D6D6D6;
  border-top: 1px solid #D6D6D6;
}
.book-navigation .page-links {
  border-top: 1px solid #D6D6D6;

Sorry, that's been driving me nuts. ;)

@droplet in #113:

- Even with the 9.8% width on the 'up', we still need to have the letter-spacing fix, tested in Chrome and Firefox.
- In which browser did you take the screenshot on the vertical-align proposal? I tested this in Chrome and Firefox, and don't have that issue, everything is aligned at the top here. Also, I don't think the vertical-align would help in this case.
- if we remove the padding altogether, and update the borders as you proposed, then we get a double border above the book pager when there is no menu above it. I have however changed the padding by only applying a top and bottom one, so that we at least inherit the left and right ones.

@mgifford in #115:
- what padding on the 'up' are you referring too? There's no padding on that element for the moment.
- Trimming the length might indeed be an option, but I would love some more opinions on this before we implement that.

fixed the problem with very long titles & top align the text.
added a badass little up arrorw to the up - to keep with the < >
moved the border colors over to bartik.css where they belong
tested in all browser i could possibly find.

i dont think theres more that can be done on this one now ;)

@#117 jyve,

- In which browser did you take the screenshot on the vertical-align proposal? I tested this in Chrome and Firefox, and don't have that issue, everything is aligned at the top here. Also, I don't think the vertical-align would help in this case.

Apply your patch #114, and browsers support inline-block have this problem, technically this issue always happen on display:inline-block
(http://blog.mozilla.com/webdev/2009/02/20/cross-browser-inline-block/)

- if we remove the padding altogether, and update the borders as you proposed, then we get a double border above the book pager when there is no menu above it. I have however changed the padding by only applying a top and bottom one, so that we at least inherit the left and right ones.

Nice, never thought it before. But by default installation, how to remove menu (.book-navigation ul.menu)? and any chance to remove book pager/nav (.book-navigation ul.page-links) ??

fixes for vertical alignment top
border moved to bartik

okay so if we have a pre & next arrow - how come that there isnt a up arrow?
the patch is a build on top of #121

screenshot is attached - or should this end up in another issue ?

i have added the add an up arrow as an seperate issue #1340242: add an arrow for the book up

I think the up arrow is weird but anyway thats a seperate issue IMO.

* cleaned up and removed the "pixel perfect" fixes, ts overkill for stark & were not winning that much by those x pixels. pixel perfect is for the ebd theme and not core + makes it easier to maintain
* changed the classe namer for the page-links to the module specific book-pager
* removed the up arrow (cause thats for another patch & discussion)

and without the pink test color *doh*

Nice work @mortendk! This is looking good. A couple of minor issues to fix before this is ready.

+++ b/core/modules/book/book-navigation.tpl.phpundefined
@@ -30,22 +29,27 @@
+          <a href="<?php print $prev_url; ?>" rel="prev" title="<?php print t('Go to previous page'); ?>"><b><?php print t('‹'); ?></b><?php print $prev_title; ?></a>

We need an empty space between the title and the b tags. The arrow is on top of the text and hard to see.

+++ b/core/modules/book/book.theme.cssundefined
@@ -0,0 +1,40 @@
+  *float:left; /* in ie7 the .page-next float:right adds a line break - unless it has float:left */

There needs to be a space after the colon, and the comment needs work. We should move the comment to the line above the code, and also include a LTR comment inline. So, like this:

  /* Prevents a line break caused by the float in .page-next in IE7. */
  *float: left; /* LTR */

+++ b/core/modules/book/book.theme.cssundefined
@@ -0,0 +1,40 @@
+  *zoom: 1;

Let's loose the * before the zoom property. We don't use that anywhere else in core.

+++ b/core/modules/book/book.theme.cssundefined
@@ -0,0 +1,40 @@
+  text-align: left;

This needs a /* LTR */ comment.

+++ b/core/modules/book/book.theme.cssundefined
@@ -0,0 +1,40 @@
+  text-align: right;
+  float: right;

Both of these need a LTR comment.

+++ b/core/themes/bartik/css/style.cssundefined
@@ -1648,3 +1648,13 @@ div.admin-panel .description {
+.book-navigation .page-links {

This should be .book-navigation .book-pager, or just .book-pager.

16 days to next Drupal core point release.

+++ b/core/themes/bartik/css/style.cssundefined
@@ -1648,3 +1648,13 @@ div.admin-panel .description {
+/* ---------- book ----------- */

and the comment format

16 days to next Drupal core point release.

here we go with another one :
* cleaned up the class names even more to be easier & quicker to read
* moved the specifics for bartik (the book-navigation .book-pager ul's margin:0 that need an overwrite now that bartik sets that as default)
* added the space between the b and link - even that i think this should be done with css & not a space - but i wont fight over it ;)
* fixed the comments for RTL
* testet in ie6-9, latest: opera,safari, chrome & ff

/me just cheerleads in here for a bit. Go team!

Tested the patch in #128:

- There's one white line too much here:

+/**
+ * Book navigation.
+ */
+
+.book-navigation .menu {

- Is there a reason why the width of the up link has been reduced to 4? The positioning is still ok for me when it is at 9% unless this causes issues in other browsers than Firefox/Chrome?

.book-pager .up {
  text-align: center;
  width: 4%;
}

- This needs to be alphabetical:

.book-pager .next {
  text-align: right; /* LTR */
  float: right; /* LTR */
  width: 45%;
}

Looking good otherwise!

its changed to 8% now, we had funny jumps with resizing to small screensizes

Patch in #131 reviewed and looks perfect to me now.

@droplet in #127: I suggest we open up a new issue to update all comments in Bartik since they don't follow the Drupal CSS conventions.

@mgifford in #132: I'm actually not really pro cutting the length anymore, unless we can make it a setting in the backend, but it feels like overkill? Any other opinions on this?

I think this is ready now. Lucky 135? :P

This looks like good cleanup. Committed/pushed to 8.x! Nice to see new people patching core in here as well.