414445 - The Back and Forward button have arrows below them in Text mode and custom themes

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P2)

Description

[Stephen Donner [:stephend] Not actively reading bugmail]

Build ID: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008012804 Minefield/3.0b3pre

In "Text" mode, chosen from the "Customize..." dialog, the "Back" button's dropdown appears in-between the "Forward" and "Reload" buttons.

See screenshot.

Comment 1

[Stephen Donner [:stephend] Not actively reading bugmail]

Screenshot: http://img20.imageshack.us/my.php?image=picture3mt3.png

Comment 2

[Simon Bünzli]

That's kind of expected as this is the unified button's dropmarker you're looking at. We need a better way of indicating that, though (although on OS X we could just hide the dropdown as we do when only icons are displayed).

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

So what about the iconic mode? Currently there is no drop marker displayed. Is this by design or do we really miss it? 

Comment 4

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #3)
> So what about the iconic mode? Currently there is no drop marker displayed. Is
> this by design or do we really miss it? 

Specifically, are we still doing http://blog.mozilla.com/faaborg/2008/01/17/keyhole-coming-to-os-x-and-windows-themes/?

Comment 5

[Alex Faaborg [:faaborg] (Firefox UX)]

Yes, if you check out toolbar.png you can see the source artwork, hasn't been implemented in css yet:

http://mxr.mozilla.org/seamonkey/source/browser/themes/pinstripe/browser/Toolbar.png

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks Alex. So all this remaining work is covered by which bug? I think that it is this one? 

Comment 7

[Alex Faaborg [:faaborg] (Firefox UX)]

I'm not sure if getting keyhole on OS X has a paticular bug, or if it is just being covered in the generic bug 397723

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #7)
> I'm not sure if getting keyhole on OS X has a paticular bug, or if it is just
> being covered in the generic bug 397723

Kevin, should we file a new bug therefor? 

Comment 9

[Alfred Kayser]

Attachment: Screenshot of the problem

In the Mac version of Firefox, the back/forward buttons still have their own dropdown buttons. See screenshot. Both of the buttons have their own dropdown marker, even if DOMinspector can't seem to find them.
The only way to remove them is to add the following rules in browser.css (hopefully):

#unified-back-forward-button>#back-button .toolbarbutton-menubutton-dropmarker,
#unified-back-forward-button>#forward-button .toolbarbutton-menubutton-dropmarker{display:none}

Comment 10

[Alfred Kayser]

Note, in the Proto theme, these dropdown markers are explicitly hidden:
http://lxr.mozilla.org/seamonkey/source/browser/themes/pinstripe/browser/browser.css#303
303 #back-button .toolbarbutton-menubutton-dropmarker,
304 #forward-button .toolbarbutton-menubutton-dropmarker {
305   display: none;
306 }

But that is strange as the back/forward buttons are defined as normal 'toolbarbutton-1' buttons, and should have no dropmakers at all.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Dropmarkers are positioned under the text

With Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b4) Gecko/2008030317 Firefox/3.0b4 ID:2008030317 the dropmarkers of back and forward are now positioned under the text.

Comment 12

[KLB]

I tried adding the following CSS instruction to my theme to turn off the dropmarkers, but was told by a Mac tester of my theme that it did not work:

#back-button > .toolbarbutton-menubutton-dropmarker,
#forward-button > .toolbarbutton-menubutton-dropmarker {
   display: none !important;
}

Comment 13

[Dão Gottwald [:dao]]

If you're using the bf-menu-button binding from pinstripe's browser.xml, then .toolbarbutton-menubutton-dropmarker is not a direct child of the buttons.
Which makes me wonder: Why are you using that binding? Or if you aren't, where's the dropmarker coming from? Please use DOM Inspector to check this.

Comment 14

[KLB]

I'm not sure who Dão Gottwald's last comment is directed at, but I tried my style instructions with and without the ">" with no effect. I can't use the DOM Inspector to dig through this because I don't have a Mac. What would be really helpful is if someone using a Mac could post a screen capture of the DOM tree showing the parent and child objects for the back/forward drop markers.  Maybe with this screen capture we could at least come up with a workable temporary fix to hide the drop markers.

Comment 15

[Dão Gottwald [:dao]]

In order to fix this bug rather than finding workarounds, it would also be interesting to see the buttons' XBL bindings and the CSS rules that assign the bindings.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: CSS style rules

Here the current CSS style rules made with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b5pre) Gecko/2008031204 Minefield/3.0b5pre ID:2008031204

For which special element you need the XML binding? The dropmarker or the forward/back button?

Comment 17

[Dão Gottwald [:dao]]

the toolbarbuttons, forward-button or back-button.

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Choosing the forward/back-button I see following binding:
chrome://browser/skin/browser.xml#bf-menu-button

Comment 20

[Mike Beltzner [:beltzner, not reading bugmail]]

(I think this bug needs a better summary, probably more like bug 419870)

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Dao, does the binding changes between icon mode and text mode? If yes, the given one in comment 18 will be wrong. I haven't switched to text mode before.

Comment 22

[Dão Gottwald [:dao]]

It's hard for me to analyze this bug in depth -- I don't have OS X. I could continue guessing by looking at pinstripe's code, but there are probably others around who could move this bug forward in a more qualified fashion.

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+2]]

There is one more thing which is odd in conjunction with the back/forward buttons in small icon mode and correlates with this bug. While the dropmarkers are shown under both buttons in text mode they are invisible in the icon mode. If you click on the empty area between the buttons and the toolbar border the context menu opens. This bothered me multiple times today because you need a separate click outside the menu to close it again

Comment 24

[KLB]

Here is some more about this bug. I added the following code to my browser.css file:

#back-button .toolbarbutton-menubutton-dropmarker,
#forward-button .toolbarbutton-menubutton-dropmarker {
  display: none !important;
}

While the code made the dropmarkers disappear, clicking on the back or forward button causes the history popup appear instead of the browser window back or forward on page.

I then changed the code to:

#back-button menupopup,
#forward-button menupopup,
 #back-button .toolbarbutton-menubutton-dropmarker,
#forward-button .toolbarbutton-menubutton-dropmarker {
  display: none !important;
}

This did cause the history popup to stop appearing, however, then the forward and back buttons then didn't do anything.

My next attempt was to dig through the default classic.jar file for Mac Firefox and see if I could figure what was different with the XML files.  What I observed was that the Mac version of the browser.xml file had a lot more stuff in it than the Windows version. I looked for any back/forward button code in the Mac version of browser.xml and tried putting it in the Windows version of browser.xul and still nothing changed.  The code I added to my theme was as follows:

<!-- Added to browser.xml below "widget" comment -->
  <binding id="bf-menu-button-allowevents-box"
           display="xul:button"
           extends="chrome://global/content/bindings/button.xml#button-base">
    <content allowevents="true">
      <children/>
    </content>
  </binding>

  <binding id="bf-menu-button" extends="chrome://global/content/bindings/toolbarbutton.xml#menu-button">
    <content>
      <children includes="observes|template|menupopup|tooltip"/>
      <xul:hbox flex="1" class="allowevents-box"/>
      <xul:vbox>
        <xul:hbox align="center" flex="1">
          <xul:toolbarbutton class="box-inherit toolbarbutton-menubutton-button"
                             anonid="button"
                             allowevents="true"
                             flex="1"
                             align="end"
                             xbl:inherits="disabled,crop,image,label,accesskey,command,
                                           dir,pack,orient,toolbarmode,buttonstyle"/>
          <xul:dropmarker type="menu-button"
                          class="toolbarbutton-menubutton-dropmarker"
                          xbl:inherits="align,dir,pack,orient,disabled,toolbarmode,buttonstyle,label"/>
        </xul:hbox>
        <xul:hbox flex="1" class="toolbarbutton-text-box allowevents-box">
           <xul:label class="toolbarbutton-text"
                      crop="right"
                      xbl:inherits="value=label,accesskey,crop,toolbarmode,buttonstyle"
                      flex="1"/>
        </xul:hbox>
      </xul:vbox>
      <xul:hbox flex="1" class="allowevents-box"/>
    </content>
  </binding>




<!-- Added to browser.xml above "globalBindings -->
<!DOCTYPE bindings [
<!ENTITY % tabBrowserDTD SYSTEM "chrome://browser/locale/tabbrowser.dtd" >
%tabBrowserDTD;
<!ENTITY % globalDTD SYSTEM "chrome://global/locale/global.dtd">
%globalDTD;
]>



<!-- Added to globalBindings .xml above "globalBindings -->
<!DOCTYPE bindings SYSTEM "chrome://global/locale/preferences.dtd">



It does not make sense that the back and forward buttons are taking on the behavior of the dropmarkers. Especially when the related XML bindings I could find were added to their relevant XML files.  What else is going on here?

For anyone who wants to test the changes I mentioned above, you can download the test version of my theme from: http://environmentalchemistry.com/classiccompact.jar  

Comment 25

[KLB]

I just realized that the XML binding instructions don't do anything unless they are referenced from the relevant CSS file.  I have added the following instructions to my browser.css file:

toolbar:not([mode="text"]) #back-button,
toolbar:not([mode="text"]) #forward-button {
  -moz-binding: url(chrome://browser/skin/browser.xml#bf-menu-button);
}

toolbar:not([mode="text"]) #back-button .toolbarbutton-menubutton-button,
toolbar:not([mode="text"]) #forward-button .toolbarbutton-menubutton-button {
  -moz-binding: url('chrome://global/content/bindings/toolbarbutton.xml#toolbarbutton-image');
}

toolbar:not([mode="text"]) #back-button .allowevents-box,
toolbar:not([mode="text"]) #forward-button .allowevents-box {
  -moz-binding: url(chrome://browser/skin/browser.xml#bf-menu-button-allowevents-box);
}


They did not cause problems in Windows, now I just need someone to test them in Mac (I've updated my classiccompact.jar file).

Comment 26

[Alfred Kayser]

To summarize, the pinstripe theme uses a special binding for the back & forward buttons: bf-menu-button. But it is not really clear why.
Possibly just removing this binding will solve this issue.

A way to try is to rebind the back- and forward-button to:
chrome://global/content/bindings/toolbarbutton.xml#toolbarbutton.

What really needs to happen is to find out why pinstripe/Mac has this weird bindings for the back/forward buttons.

Comment 27

[Dão Gottwald [:dao]]

Note that the dropmarker has been removed in bug 423361, but as Alfred says, the binding should be removed altogether if it's not really needed.

Comment 28

[KLB]

My Mac tester did confirm that the binding changes did fix the back/forward buttons.  To summarize my notes above, the following changes are needed to a Windows based theme to disable the "stray" back and forward dropmarkers and to make the back/forward buttons behave correctly:

==== XML Binding changes ====

<!-- Add to browser.xml below "widget" comment -->
  <binding id="bf-menu-button-allowevents-box"
           display="xul:button"
           extends="chrome://global/content/bindings/button.xml#button-base">
    <content allowevents="true">
      <children/>
    </content>
  </binding>

  <binding id="bf-menu-button"
extends="chrome://global/content/bindings/toolbarbutton.xml#menu-button">
    <content>
      <children includes="observes|template|menupopup|tooltip"/>
      <xul:hbox flex="1" class="allowevents-box"/>
      <xul:vbox>
        <xul:hbox align="center" flex="1">
          <xul:toolbarbutton class="box-inherit
toolbarbutton-menubutton-button"
                             anonid="button"
                             allowevents="true"
                             flex="1"
                             align="end"
                            
xbl:inherits="disabled,crop,image,label,accesskey,command,
                                          
dir,pack,orient,toolbarmode,buttonstyle"/>
          <xul:dropmarker type="menu-button"
                          class="toolbarbutton-menubutton-dropmarker"
                         
xbl:inherits="align,dir,pack,orient,disabled,toolbarmode,buttonstyle,label"/>
        </xul:hbox>
        <xul:hbox flex="1" class="toolbarbutton-text-box allowevents-box">
           <xul:label class="toolbarbutton-text"
                      crop="right"
                     
xbl:inherits="value=label,accesskey,crop,toolbarmode,buttonstyle"
                      flex="1"/>
        </xul:hbox>
      </xul:vbox>
      <xul:hbox flex="1" class="allowevents-box"/>
    </content>
  </binding>


=== CSS Changes ===

/* Add to browser.css */
 
#back-button .toolbarbutton-menubutton-dropmarker,
#forward-button .toolbarbutton-menubutton-dropmarker {
  display: none !important;
}

toolbar:not([mode="text"]) #back-button,
toolbar:not([mode="text"]) #forward-button {
  -moz-binding: url(chrome://browser/skin/browser.xml#bf-menu-button);
}

toolbar:not([mode="text"]) #back-button .toolbarbutton-menubutton-button,
toolbar:not([mode="text"]) #forward-button .toolbarbutton-menubutton-button {
  border-left: 0;
  border-right: 0;
}
toolbar:not([mode="text"]) #back-button .allowevents-box,
toolbar:not([mode="text"]) #forward-button .allowevents-box {
  -moz-binding: url(chrome://browser/skin/browser.xml#bf-menu-button-allowevents-box);
}



To answer Alfred Kayser's question about the strange back/forward binding, I think it may have to do with the fact that Mac Firefox still provides independent history drop down menus for the back and forward buttons if one right mouse clicks on these buttons or if the click and hold on the button.

Again the full demo of the changes I mention above can be tried at http://EnvironmentalChemistry.com/classiccompact.jar

Comment 29

[Alfred Kayser]

See bug 423361 where they are also noting that the dropmarkers should NOT be there anymore. The new pinstripe (Pronto 0.14) even also does a 'display:none' on these.

Comment 30

[Henrik Skupin [:whimboo][⌚️UTC+2]]

The dropmarkers are definitely there. I compiled a fresh cvs build some minutes ago and did the same thing I mentioned in comment 23. The context menu still occurs when clicking on the area between the dropmarkers and the toolbar border. 

Comment 31

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Sorry I meant the back/forward buttons. The dropmarkers aren't visible.

Comment 32

[Alfred Kayser]

The bit in the patch of bug 423361 that 'display:none' these dropmarkers is:
-#back-button .toolbarbutton-menubutton-dropmarker,
-#forward-button .toolbarbutton-menubutton-dropmarker {
+#back-button > dropmarker,
+#forward-button > dropmarker {
   display: none;
 }

Unfortunelately one cannot just add:
#back-button > dropmarker,
#forward-button > dropmarker {
   display: none;
}
When one wants to keep backwards compatibility with FF2.

But the following might work:
#unified-back-forward-button #back-button > dropmarker,
#unified-back-forward-button #forward-button > dropmarker {
   display: none;
}

Who can test this on a Mac?

Comment 33

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I could. But where do I have to make the changes?

Comment 34

[Dão Gottwald [:dao]]

(In reply to comment #32)
> +#back-button > dropmarker,
> +#forward-button > dropmarker {
>    display: none;
>  }

Umm, I don't think these selectors matches anything ... can we just remove them?

(In reply to comment #33)
> I could. But where do I have to make the changes?

In pinstripe's browser.css

Comment 35

[Alfred Kayser]

Attachment: A theme with hopefully the fix for the dropdown buttons

Can someone test this on a Mac? 

The real issue is that the Mac platform for some silly reason, even if Pinstripe is not used, as different bindings for these back/forward buttons than Windows and Linux.

Comment 36

[Henrik Skupin [:whimboo][⌚️UTC+2]]

How do I manually install a theme? Please also give me steps I have to follow to see what should have been changed. 

Comment 37

[Alfred Kayser]

Very simple: Just drag&drop the jarfile (this can even be done from a link on a website) into the Addons manager. After installation, select the theme 'LittleFox' and restart FF.

If all is well, the back/forward buttons should have no 'dropdown' markers beneath them.

Comment 38

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Sorry for the delay. When trying to test this now shows me that there was a change on trunk. With the latest nightly build the dropmarkers are gone on OS X. They aren't visible anymore when choosing text mode.

Comment 39

[Mike Connor [:mconnor]]

Attachment: remove the unnecessary binding, fix code to deal.

Remove the old crufty bindings, they're not needed.  This seems to fix a lot of stuff, including bug 414470 which is also a blocker.

Comment 40

[Steve Swonk]

I am currently running Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b5pre) Gecko/2008032604 Minefield/3.0b5pre and Littlefox 1.8.26. The display of the back-forward buttons has been fixed in Littlefox. However, the ultimate issue of the buttons' behavior is still not working properly.

On the Mac, the expected behavior of these buttons is:

Click = Immediately go back or forward one page
Click and Hold = display back or forward history in a drop down list
Control key (contextual menu display) + Click = display back or forward history in a drop down list

Using the Firefox default theme, this behavior is as expected. However, when using Littlefox 1.8.26, Click = "display back or forward history in a drop down list" as if the button were held down or modified with the Mac Control key. Simply clicking should immediately move one page back or forward but it does not with the latest Littlefox.

I have discussed this with Alfred, and we are still concerned that the Mac bindings are not reliable.

Comment 41

[Asaf Romano (gone)]

Comment on attachment 311920 [details] [diff] [review]
remove the unnecessary binding, fix code to deal.

finally!

r=mano.

Comment 42

[Mike Connor [:mconnor]]

Steve, the problem is that the behaviour depends on the bindings, and the bindings aren't needed.  Please retest on tomorrow's build, I just checked in the fix for this.

Checking in browser/base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.1017; previous revision: 1.1016
done
Checking in browser/themes/pinstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/pinstripe/browser/browser.css,v  <--  browser.css
new revision: 1.138; previous revision: 1.137
done
Checking in browser/themes/pinstripe/browser/browser.xml;
/cvsroot/mozilla/browser/themes/pinstripe/browser/browser.xml,v  <--  browser.xml
new revision: 1.10; previous revision: 1.9
done

Comment 43

[Steve Swonk]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008040304 Minefield/3.0pre using Littlefox 1.8.26, the back and forward buttons are now working as expected.

Comment 44

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008040504 Minefield/3.0pre ID:2008040504

Alex, the back/forward text in text mode is vertically not aligned. It's positioned too high. Is there already a bug about that issue? I cannot find one. 

Comment 45

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #44)
> Alex, the back/forward text in text mode is vertically not aligned. It's
> positioned too high. Is there already a bug about that issue? I cannot find
> one. 

Sorry I meant *each* text of any added toolbar buttons.

Comment 46

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I filed bug 427418 therefor.