54.10 KB, image/png
55.04 KB, image/png
356 bytes, text/html
1.17 KB, patch
|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
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.
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.
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.
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]
minimized testcase: https://builder.addons.mozilla.org/addon/1000076/latest/
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.
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.
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
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
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
Created attachment 515655 [details] [diff] [review] Remove max-height 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.
Created attachment 515665 [details] [diff] [review] Remove max-height and center iframe Same patch but align widget on center.
Comment on attachment 515665 [details] [diff] [review] Remove max-height and center iframe r=me, tested fine on Mac and Linux.
Followup patch (attachment 515665 [details] [diff] [review]) landed: https://github.com/mozilla/addon-sdk/commit/24c8f4a67a76e0aaac7fc4ebcde62492c2f4c2ce