964721 - For type="menu-button" buttons, the dropmarker can keep 'active' styling when closing the panel by clicking on web content.

Status: RESOLVED WORKSFORME

(Firefox :: Theme, defect)

Description

[:Gijs (he/him)]

(In reply to dw-dev from bug 940307 comment #65)
> Please see attachment 8366515 [details], attachment 8366516 [details], attachment 8366517 [details] & attachment 8366518 [details].
> 
> I don't think [the appearance of menu buttons in the panel] is fixed.
> 
> The styling is much improved, BUT the positioning of the icon and text label
> is ONLY correct if the text label wraps into a second line.
> 
> If the text label fits on one line, then both the icon and the text label
> are displayed too high, and the icon extends outside the button.
> 
> I have tested this with Print Edit's button and Download Them All's buttons.

This should really be a separate bug as the majority of the other bug is fixed, so I've filed this one.

> Also, if you click on the dropmarker to show the menu, and then click on the
> menu panel background, the menu closes as expected, BUT the dropmarker is
> still highlighted.

That's because if you do this, the dropmarker is still :active. It is never :hover because it's an anon element that doesn't have mouse events. I'm not sure how fixable this is, but I can see if I have time to have a look.

The overflow list issues you raised already have their own bug and don't need to be dealt with neither here nor in bug 940307.

Generally, please please please keep one bug per issue; unless a bug is completely not fixed at all, sticking more information in that bug about follow-up issues is not useful; both for dependencies tracking and prioritizing the followup issues putting too many things in one bug is confusing and problematic. (a rule that I'm kind of already violating with this bug as the dropmarker and the single-line labels are separate issues)

Now, separately, considering the size of our backlog, it'd be very helpful if someone could try to produce a patch for this issue, or at least produce more diagnostics on what's wrong (in particular, a difference in the margins, paddings and height of a single-line and multiline menubutton's toolbarbutton-menubutton-button, its toolbarbutton-icon and toolbarbutton-multiline-text); I don't know how much time I can find to work on this particular issue considering the magnitude of the other blocking bugs and the schedule pressure we're under. :-(

The patches in bug 940307 should give a good idea of where to look for the code that'd need fixing.

Comment 1

[Drew Willcoxon :adw]

Attachment: on OS X

I've been looking at this and so far haven't made much progress, but on OS X with a freshly pulled tree, the buttons look different from those in the attachments.  In the two buttons with a single line of text, the icons are stretched horizontally.  The text of the Print Edit button looks slightly too tiny.  And the DownThemAll button isn't centered horizontally.

Comment 2

[dw-dev]

On Windows, with the latest Nightly, the Print Edit and Download Them All buttons in the menu panel still look the same as screen shot attachments I submitted.

Note, however, my screen shots were made with a test version of Print Edit which does not specifically set the text font-size, whereas the currently released version of Print Edit sets the text font-size to 10px.

Comment 3

[:Gijs (he/him)]

Greeeeaaat. So first off, some confusion:

freshly pulled tree != latest nightly.

Drew's clearly has the patch for bug 897496. Latest nightly doesn't.


Other than that... it's a bit strange because in bug 897496 we tried to be careful to style type="menu-button" appropriately. I wonder what's breaking things for the specific buttons. We don't set font sizes, for example.

Comment 4

[:Gijs (he/him)]

(In reply to dw-dev from comment #2)
> On Windows, with the latest Nightly, the Print Edit and Download Them All
> buttons in the menu panel still look the same as screen shot attachments I
> submitted.
> 
> Note, however, my screen shots were made with a test version of Print Edit
> which does not specifically set the text font-size, whereas the currently
> released version of Print Edit sets the text font-size to 10px.

Oh. I missed this in my groggy still-half-asleep state. That explains the font-size. So that part should be fixed then. Hurray? I wonder what's up with the icons. I /suspect/ they set margins on the icon and that now messes with things, but we should really doublecheck.

Comment 5

[dw-dev]

I've done some more investigation and found out what the problem is with single-line text type="menu-button" buttons.

The styling on the inner xul:toolbarbutton is currently "-moz-box-pack:center".  When this is changed to "-moz-box-pack:start", single-line text buttons and multi-line text buttons both display correctly.

I also noticed a separate problem - the "DownThemAll!..." button has the drop marker offset to the right and extending outside of the button.  This is caused by the single long word in the text label.  This was easily fixed by changing the text to "Down Them All!...".

Comment 6

[:Gijs (he/him)]

(In reply to dw-dev from comment #5)
> I've done some more investigation and found out what the problem is with
> single-line text type="menu-button" buttons.
> 
> The styling on the inner xul:toolbarbutton is currently
> "-moz-box-pack:center".  When this is changed to "-moz-box-pack:start",
> single-line text buttons and multi-line text buttons both display correctly.
> 
> I also noticed a separate problem - the "DownThemAll!..." button has the
> drop marker offset to the right and extending outside of the button.  This
> is caused by the single long word in the text label.  This was easily fixed
> by changing the text to "Down Them All!...".

I believe both of these issues should be fixed now without any further adjustments from the add-on's sides. Is my assumption correct?

Comment 7

[dw-dev]

Testing with Nightly 2014-02-11.

The problem of incorrect positioning of the icon and text for single-line text type="menu-button" buttons has been fixed.

The problem of the dropmarker remaining highlighted has been fixed for most cases.  If you click on the dropmarker to show the menu, then click on the menu panel background, the menu closes and the highlighting of the dropmarker is removed (as expected).  However, if you click on the dropmarker to show the menu, then click on the page contents, the menu panel closes but the highlighting of the dropmarker occasionally remains (not as expected - it seems to depend on the path taken by the mouse when moving from the dropmarker onto the page contents).

Comment 8

[:Gijs (he/him)]

(In reply to dw-dev from comment #7)
> Testing with Nightly 2014-02-11.
> 
> The problem of incorrect positioning of the icon and text for single-line
> text type="menu-button" buttons has been fixed.

Excellent, thanks for confirming that!

> The problem of the dropmarker remaining highlighted has been fixed for most
> cases.  If you click on the dropmarker to show the menu, then click on the
> menu panel background, the menu closes and the highlighting of the
> dropmarker is removed (as expected).  However, if you click on the
> dropmarker to show the menu, then click on the page contents, the menu panel
> closes but the highlighting of the dropmarker occasionally remains (not as
> expected - it seems to depend on the path taken by the mouse when moving
> from the dropmarker onto the page contents).

That seems like a minor issue. At this stage, I would take a patch, but I don't think it should be something we rush to fix for Firefox 29, so I'm downgrading the priority of this.

Comment 9

[Kathleen :Brade]

> The problem of the dropmarker remaining highlighted has been fixed for most
> cases.  If you click on the dropmarker to show the menu, then click on the
> menu panel background, the menu closes and the highlighting of the
> dropmarker is removed (as expected).  However, if you click on the
> dropmarker to show the menu, then click on the page contents, the menu panel
> closes but the highlighting of the dropmarker occasionally remains (not as
> expected - it seems to depend on the path taken by the mouse when moving
> from the dropmarker onto the page contents).

I also see this problem when I choose an item from the menu that is attached to the menu-button.  This seems like a very common case.  The menu panel closes after I choose a menu item but the dropmarker is still highlighted the next time I re-open the menu panel.  Should I open a new bug for this case?

Comment 10

[:Gijs (he/him)]

We no longer support custom buttons here, so we can close this.