942051 - Toolbar button with long text cannot be added due to wrong width measurement

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Wladimir Palant]

Adblock Plus calls toolbar.insertItem() to add its item to the add-on bar. Now that Australis landed this produces the error message: "abp-toolbarbutton is too big (107px wide), moving to palette" (the message is coming from function evictNode() in browser/components/customizableui/content/toolbar.xml). The catch: this is an absolutely standard toolbar button with a 16x16 icon.

As it turns out, the toolbar button text (which is normally invisible) is being considered for the width measurement. That's because the size is measured after the icon is added to the fake add-on bar - and unlike the main toolbar it doesn't have iconsize or mode attributes. This means that toolbar buttons show up with large icon and text, this makes them significantly larger than when they appear in the navigation toolbar.

Adding iconsize="small" and mode="icons" to <toolbar id="addon-bar"> fixes the issue, the button is no longer rejected. I've been testing in Firefox 28.0a1 (2013-11-21 nightly) on Linux.

Comment 1

[:Gijs (he/him)]

(In reply to Wladimir Palant from comment #1)
> As it turns out, the toolbar button text (which is normally invisible) is
> being considered for the width measurement. That's because the size is
> measured after the icon is added to the fake add-on bar - and unlike the
> main toolbar it doesn't have iconsize or mode attributes. This means that
> toolbar buttons show up with large icon and text, this makes them
> significantly larger than when they appear in the navigation toolbar.

Can you reproduce this problem on a clean profile? What styles does ABP itself add? If not, I'm blaming bug 940953.

Comment 2

[Wladimir Palant]

Yes, I can reproduce it on a clean profile - this has nothing to do with the attributes of the nav-bar element, it's rather about addon-bar. You can reproduce it with the following steps without any extensions whatsoever:

1. Make sure devtools.chrome.enabled preference is set to true.
2. Open Scratchpad (Shift-F4).
3. Click Environment menu of the scratchpad and select Browser.
4. Paste the following code into the text field:

{
    let button = document.createElement("toolbarbutton");
    button.setAttribute("id", "my-test-test-button");
    button.setAttribute("label", "Some long button label");
    button.setAttribute("image", "https://bugzilla.mozilla.org/extensions/BMO/web/images/favicon.ico");

    let toolbar = document.getElementById("addon-bar");
    toolbar.toolbox.palette.appendChild(button);
    toolbar.insertItem("my-test-test-button");
}

5. Execute the code (Ctrl-R).

Expected result:

A new toolbar item appears.

Actual result:

Nothing happens, Browser Console lists the following error:

> Error: my-test-test-button is too big (141px wide), moving to the palette

Note that using a shorter label makes the new toolbar item appear. Also, its size is completely independent of the label (which isn't visible in the main toolbar) - 30x36. The label is only visible in the fake addon-bar toolbar where the button's size is measured.

Comment 3

[:Gijs (he/him)]

(In reply to Wladimir Palant from comment #2)
> Yes, I can reproduce it on a clean profile - this has nothing to do with the
> attributes of the nav-bar element, it's rather about addon-bar.

Note that the configurable part of iconsize was always set on the toolbox, not the bar itself, hence my question.

> You can
> reproduce it with the following steps without any extensions whatsoever:
> 
> 1. Make sure devtools.chrome.enabled preference is set to true.
> 2. Open Scratchpad (Shift-F4).
> 3. Click Environment menu of the scratchpad and select Browser.
> 4. Paste the following code into the text field:
> 
> {
>     let button = document.createElement("toolbarbutton");
>     button.setAttribute("id", "my-test-test-button");
>     button.setAttribute("label", "Some long button label");
>     button.setAttribute("image",
> "https://bugzilla.mozilla.org/extensions/BMO/web/images/favicon.ico");
> 
>     let toolbar = document.getElementById("addon-bar");
>     toolbar.toolbox.palette.appendChild(button);
>     toolbar.insertItem("my-test-test-button");
> }
> 
> 5. Execute the code (Ctrl-R).
> 
> Expected result:
> 
> A new toolbar item appears.
> 
> Actual result:
> 
> Nothing happens, Browser Console lists the following error:
> 
> > Error: my-test-test-button is too big (141px wide), moving to the palette
> 
> Note that using a shorter label makes the new toolbar item appear. Also, its
> size is completely independent of the label (which isn't visible in the main
> toolbar) - 30x36. The label is only visible in the fake addon-bar toolbar
> where the button's size is measured.

It actually WFM on OS X I even tried increasing the length of the label to ensure it wasn't a font issue. Trying on Windows next, maybe the styling is somehow different between them, but then this shouldn't be all/all.

Comment 4

[:Gijs (he/him)]

Attachment: should set mode=icons and iconsize=small on the add-on bar for Australis,

I think I may have messed up trying to reproduce on mac by not opening a new profile, because old profiles might actually be persisting the correct value. See also bug 940953, reversed. Anyhow, this patch is simple, and fixes the issue as noted in the scratchpad.

Comment 5

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/integration/fx-team/rev/886109223667

Comment 6

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/886109223667