Closed Bug 626728 Opened 15 years ago Closed 15 years ago

toolbar changes its size when adding an addon button

Categories

(Add-on SDK Graveyard :: General, defect)

Product:
Add-on SDK Graveyard
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: jorge.es.iu, Unassigned)

References

(
URL
)

Details

Attachments

(4 files, 1 obsolete file)

Urlbar showing the bug
54.10 KB, image/png
Details
Tab bar showing the bug
55.04 KB, image/png
Details
Pointer to pull request
356 bytes, text/html
myk
: review+
myk
: review+
Details
Remove max-height and center iframe
1.17 KB, patch
dietrich
: review+
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b10pre) Gecko/20110118 Firefox/4.0b10pre Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b10pre) Gecko/20110118 Firefox/4.0b10pre When you insert some addon buttons to tab bar or url bar, its size is reduced and it doesn't look good. Reproducible: Always Steps to Reproduce: 1. Download this addon for example: https://addons.mozilla.org/firefox/addon/hard-blockers-counter/ 2. Add the addon button on the addon bar to the tab bar or the url bar. Actual Results: The size of the bar in which is the button resizes and doesn't look well. Expected Results: The icon shold be resized or it should maintein its size but the tab bar or urlbar shoul have a minimun height
Attached image Urlbar showing the bug —
Attached image Tab bar showing the bug —
Confirmed on Windows 7 Mozilla/5.0 (Windows NT 6.1; rv:2.0b10pre) Gecko/20110118 Firefox/4.0b10pre
Confirm again. Mozilla/5.0 (Windows NT 6.1; rv:2.0b10pre) Gecko/20110118 Firefox/4.0b10pre ID:20110118030327
This is a bug in the widget used by the add-on.
Component: Toolbars → Jetpack SDK
Product: Firefox → Mozilla Labs
QA Contact: toolbars → jetpack-sdk
Component: Jetpack SDK → General
Product: Mozilla Labs → Add-on SDK
QA Contact: jetpack-sdk → general
This is caused by the max-height on the toolbaritem.
This is pretty bad actually, and would very very likely be a softblocker if this was a Firefox bug.
Status: UNCONFIRMED → NEW
Ever confirmed: true
I don't see an easy way to fix that from inside of the jetpack, so there seems to be no workaround :( Also, I had to manually define the width of the widget - without it the widget content got cropped, so it seems like there's a generic CSS styling issue with the content. Last thing, from Limi: Use: 'font-family: "Segoe UI", Tahoma, "Lucida Grande", sans-serif;' to match Firefox theme.
Limi reported the font issue as bug 627189.
OS: Mac OS X → All
Hardware: x86_64 → All
This affects the Addon Bar, too, as reported in Bug 627235 (duped in comment 10). From that bug: - normal screenshot: attachment 505253 [details] - buggy screenshot: attachment 505254 [details]
Hrm. Without max-height, the toolbaritem always expands to 22px, even if I restrict the iframe within it to 16px (the iframe is sized correctly, but the toolbaritem expands vertically anyway).
Is it bad that the toolbaritem height is 22px? The iframe is still 16px high, isn't it?
(In reply to comment #14) > Is it bad that the toolbaritem height is 22px? The iframe is still 16px high, > isn't it? Fine on regular toolbars. The Add-on bar is 18px high (like the status bar was), and the unit test is checking that.
Removing the max-height didn't change the Add-on bar height for me on Mac.
This fixed it for me, using latest SDK source: https://github.com/mozilla/addon-sdk/pull/97.
Someone in #jetpack asked about the bug, tested this fix, confirmed that it worked for them on Win7 w/ beta 9.
Attachment #505782 - Flags: review?(myk)
Attachment #505782 - Flags: review?(myk) → review+
I tested the patch on macos and it does indeed seem to fix the issue with height. The remaining issues are: * if you don't specify the width of the widget, it'll get cropped * restarting the browser resets the widget positioning Do we want to cover that in this bug or file separate ones?
(In reply to comment #20) > The remaining issues are: > > * if you don't specify the width of the widget, it'll get cropped > * restarting the browser resets the widget positioning > > Do we want to cover that in this bug or file separate ones? One issue per bug please. Anything else and bugs become a mess.
https://github.com/mozilla/addon-sdk/commit/0f9e14ea9d9f3251cbe925e2b69facd31376be8a
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Blocks: 628022
No longer blocks: 628022
I backed this out, as it broke a test (bug 628598) and landed without approval in a frozen tree. https://github.com/mozilla/addon-sdk/commit/044023ee9f174bdf35e36426e8855f662006c31d
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Dietrich: any thoughts on getting this working cross-platform?
It's not necessarily a platform problem, since a bunch of the reported failures in bug 628598 are on Linux, where I tested. I'll try to spend some time looking at it this week. Alternatively, anyone else can look in the meantime :)
Comment on attachment 505782 [details] Pointer to pull request Further testing reveals that specifying both height and max-height makes tests pass on all platforms. re-r=myk with that change
Attachment #505782 - Flags: review+
Patch landed with change requested in review comment 26 in the interest of getting this in before the 1.0b3 freeze: https://github.com/mozilla/addon-sdk/commit/f0766efeebe408b4f7e9eeacfdbfe43e92f50847
Status: REOPENED → RESOLVED
Closed: 15 years ago → 15 years ago
Resolution: --- → FIXED
Attached patch Remove max-height (obsolete) — — Splinter Review
As Cork highlighted on irc, there is still such issues with widgets and addon-bar/toolbars. We should not use max-height in toolbars because it resize all the toolbar due to xul box layout. So I removed max-height on toolbaritem and add a new one on the iframe. As toolbaritem has overflow:hidden, toolbaritem size is not related to iframe one. So everything should be fine now. widget tests pass on windows.
Attachment #515655 - Flags: review?(dietrich)
Same patch but align widget on center.
Attachment #515655 - Attachment is obsolete: true
Attachment #515665 - Flags: review?(dietrich)
Attachment #515655 - Flags: review?(dietrich)
Comment on attachment 515665 [details] [diff] [review] Remove max-height and center iframe r=me, tested fine on Mac and Linux.
Attachment #515665 - Flags: review?(dietrich) → review+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: