Closed Bug 674744 Opened 9 years ago Closed 8 years ago

Implement conditional forward button for pinstripe

Categories

(Firefox :: Theme, enhancement)

All
macOS
enhancement
Not set

Tracking

()

RESOLVED FIXED
Firefox 10

People

(Reporter: jaws, Assigned: jaws)

References

Details

(Whiteboard: [fixed-in-fx-team])

Attachments

(2 files, 26 obsolete files)

31.06 KB, image/png
Details
10.02 KB, patch
dao
: review+
shorlander
: ui-review+
Details | Diff | Splinter Review
The Back button should be connected to the URL bar. The Forward button will be visually placed within the URL bar and will slide under the Back button when there is no forward navigation available.
Attached patch WIP for bug 674744 (obsolete) — Splinter Review
This is the current WIP of a patch for this bug. This WIP is only for the Winstripe theme with mode=icons and iconsize=large
Assignee: nobody → jwein
Status: NEW → ASSIGNED
Technical note: It seems that this should be doable with the back/forward buttons kept as the location bar's siblings, rather than children of the location bar.
(In reply to comment #2)
> Technical note: It seems that this should be doable with the back/forward
> buttons kept as the location bar's siblings, rather than children of the
> location bar.

It would be great to keep them as siblings, as it would solve many of the issues that we would encounter.

I could probably use positioning to place the unified-back-forward button on top of the address bar, but then animating the forward button won't trigger the URL to move. Do you have any recommendations as to how to keep them siblings and continue to make the URL animate with the forward button?
It sounds like you want a second parallel transition that changes the urlbar's left padding or something like that.
(In reply to comment #4)
> It sounds like you want a second parallel transition that changes the
> urlbar's left padding or something like that.

Yes. I'm interested if you had a clever way to do this?

I'm currently using the disabled attribute to transition the forward button, but the urlbar currently doesn't receive an attribute like this. I can look in to adding one, but I wasn't sure I'm not sure if that is the wrong direction.
There could be a "forwarddisabled" attribute on unified-back-forward-button (the forward button's parent node), and then you'd use #unified-back-forward-button[forwarddisabled] + #urlbar-container > #urlbar.
(In reply to comment #6)
> There could be a "forwarddisabled" attribute on unified-back-forward-button
> (the forward button's parent node), and then you'd use
> #unified-back-forward-button[forwarddisabled] + #urlbar-container > #urlbar.

Great idea! Thanks :)
Considering these mockups http://people.mozilla.com/~shorlander/ux-presentation/ux-presentation.html , and the OneLiner addon which Mozilla Labs published this week , i believe this should be implemented for trial in UX builds.
Attached patch WIP for bug 674744 (obsolete) — Splinter Review
This is a basic implementation of the conditional forward button. There are some rough edges but all functionality should be present.

Known issues are:
- Jankiness when hovering over the back button when the forward button is missing.
- Customize dialog looks janky when buttons are added between the unified back forward button, but the jankiness will go away when the Customize dialog is closed.
- Lack of support for Linux.
- Lightweight themes (personas) on Mac cause the url bar to show through the Back button.
Attachment #548943 - Attachment is obsolete: true
Attached patch WIP for bug 674744 v3 (obsolete) — Splinter Review
Fixed the jankiness when hovering over the back button if the forward button is disabled.
Attachment #550234 - Attachment is obsolete: true
Attached patch WIP for bug 674744 v4 (obsolete) — Splinter Review
Known issues at this point:
- Lack of "back-button glow" on hover.
- Opening the Customize dialog can cause the disabled look-and-feel of the back/forward button to get out of sync.
Attachment #550278 - Attachment is obsolete: true
The basic functionality and look-and-feel of the conditional forward button has been implemented and can be seen in UX builds.

Here is the list of known issues at this point:
- If the bookmarks button is placed between the back/forward button and the urlbar, with the addition of the bookmarks toolbar enabled, then the forward button will not be conditional. A follow-up bug will need to be filed for this case.
- Lack of support for Linux. Linux support will have to wait until we get the theme improvements for gnomestripe to add in a rounded back button, etc.
- The back button does not have the background-image glow like other toolbar buttons. This is because I have made the back button fully opaque. Additionally, the back button may look slightly different than other toolbar buttons due to its opaqueness.
- The patch-so-far uses a graphic that I made, Toolbar-disabled.png, which includes a faded-out large back arrow. This is the only element of the graphic that is being used currently, and it is because we cannot use transparency on the back button.
- Opening the Customize dialog can cause the disabled look-and-feel of the back/forward button to get out of sync. Opening a new tab or restarting the browser seems to be the only way to fix this issue at the moment.
- The site identity block has been removed for the time being. This is not a permanent change, as it has been removed until we have a good idea of how we want to display site identity information.
Is it going to be implemented for small icons mode too?
If this is the bug being pushed into UX branch for about three times in 3 days then I have an issue : 
When any button is before the unified forward back button , and urlbar is next to unified back forward button , then do the following : 
1. have a page in history to go forward to , but nothing to go back to 
2. press forward and forward button hides . 
3. press back 
4. press forward now and he urlbar go behind the back button far left to the screen edge
(In reply to scrapmachines from comment #14)

Confirmed, I can reproduce that using the provided steps. Adding or removing an element behind the back button makes the URL bar move a for a "spot" too far to the left. Restarting the browser seems to fix it though.
I second the question about small icons from comment 13.
Any plans on that, guys?
(In reply to bogas04 from comment #13)
> Is it going to be implemented for small icons mode too?

(In reply to Edward Forreal from comment #16)
> I second the question about small icons from comment 13.
> Any plans on that, guys?

I'm sorry, but as far as I understand, this will only be implemented for large icon mode.

If you would like this to be implemented for small icon mode, please file a bug to request it for small icons.
(In reply to Jared Wein [:jwein] from comment #17)

Thanks Jared. Filed Bug 677860.
Attached image save password prompt covers url (obsolete) —
I noticed a bug when the save password prompt appears: the "arrow icon" for it partially covers the beginning of the url in the location bar.
(In reply to Theodore from comment #20)
> Created attachment 552664 [details]
> save password prompt covers url
> 
> I noticed a bug when the save password prompt appears: the "arrow icon" for
> it partially covers the beginning of the url in the location bar.

Thank you for letting me know about this. I will see what I can do to fix that.
When you set small icons and set big icons back the forward button is always shown.
Not sure if its known, but this has problems with personas on osx:

http://dl.dropbox.com/u/72157/screeny.png
popup windows lacking back button have shifted location bar problem
Screenshot of the problem pointed out in comment 24.
(In reply to Darren Kalck [:D-Kalck] from comment #22)
(In reply to Wesley Johnston (:wesj) from comment #23)
(In reply to bogas04 from comment #24)
(In reply to Theodore from comment #25)

Thank you for the great feedback. I have noted these issues and they should be fixed in a new UX branch build within a couple days.
Summary: Move the back button next to the URL bar and only show forward button when useful → (Conditional forward button) - Move the back button next to the URL bar and only show forward button when useful
Attached patch WIP for bug 674744 v6 (obsolete) — Splinter Review
fryn: Do you have any tips to get around my "broadcasteventhax"? I'm trying to sync the forward navigation disabled event to other elements, but I want to rename the attribute in the process.

Also, any other general feedback you may have would be greatly appreciated :)
Attachment #552574 - Attachment is obsolete: true
Attachment #554466 - Flags: feedback?(fryn)
Attached patch WIP for bug 674744 v8 (obsolete) — Splinter Review
fryn: I have removed the previous hack from the older patch. Can you please give me feedback?
Attachment #554466 - Attachment is obsolete: true
Attachment #554638 - Attachment is obsolete: true
Attachment #554466 - Flags: feedback?(fryn)
Attachment #555199 - Flags: feedback?(fryn)
Attached video WIP v8 on Snow Leopard (obsolete) —
Jared, here's a screen recording of WIP v8 on Snow Leopard.
Note the asymmetric identity box, the distorted (lengthened) forward button, the jitter of the animation, and the slight difference in toolbar item widths between the before/after states.

The patch also regresses startup performance by causing the UI to have initial ephemeral glitches and to require several reflows after the window is painted to settle in its final state. Like the combined reload/stop/go in the url bar, the conditional forward button should be in its correct state (for default configurations) by the time the window is first painted.

I'll provide code feedback shortly.
The awesome bar suggestions start from below the Back button, which a deviation from the mockups (https://wiki.mozilla.org/images/a/a8/LocationBar-Results-i01.jpg). Please fix this.
(In reply to sdrocking from comment #29)
> Created attachment 554797 [details]
> Back button overlaps urlbar suggestions

(In reply to sdrocking from comment #32)
> The awesome bar suggestions start from below the Back button, which a
> deviation from the mockups
> (https://wiki.mozilla.org/images/a/a8/LocationBar-Results-i01.jpg). Please
> fix this.

Thanks sdrocking for letting me know about this. I have it on my TODO list to fix it :)
Also , in the latest UX builds (hourly) . I noticed a glitch .
When Home button is between unified back/forward button and urlbar, Clicking on Cusomize or toolbar layout (ie trying to change the position of unified back/forward button ) fails and nothing happes, except the disabling of customize and toolbar layout option.
Customize/Toolbar Layout works if unified back/forward button is already just before urlbar.
Since UX cset 649d951c0c86. I'm unable to customize toolbar if 'Use Small Icons' enabled.

STR
1. start with clean profile.
2. customize toolbar, checked 'Use small icons'.
3. customize toolbar again

the customize toolbar window doesn't appear and i get following error.

Error: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIObserverService.removeObserver]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://browser/content/browser.js :: uninit :: line 10555"  data: no]

I cannot reproduce this on cset 60b0b3ad4dd3.
(In reply to scrapmachines from comment #34)
> Also , in the latest UX builds (hourly) . I noticed a glitch .
> When Home button is between unified back/forward button and urlbar, Clicking
> on Cusomize or toolbar layout (ie trying to change the position of unified
> back/forward button ) fails and nothing happes, except the disabling of
> customize and toolbar layout option.
> Customize/Toolbar Layout works if unified back/forward button is already
> just before urlbar.

i second that
(In reply to scrapmachines from comment #34)
(In reply to Ek from comment #35)
(In reply to bogas04 from comment #36)

Thank you for your detailed steps to reproduce scrapmachines and Ek. I believe I have fixed this locally and will try to push out a new patch to the UX build tomorrow.
Winstripe spun off to bug 682534. The patch there adds a forwarddisabled attribute on unified-back-forward-button, which should be used here.
Severity: normal → enhancement
Depends on: 682534
OS: All → Mac OS X
Hardware: x86_64 → All
Summary: (Conditional forward button) - Move the back button next to the URL bar and only show forward button when useful → Implement conditional forward button for pinstripe
Attached patch WIP of patch for bug 674744 (obsolete) — Splinter Review
This is an updated version of the patch that only applies to pinstripe. These are the known issues:
1. The toolbar items shift when the forward button appears/disappears.
2. The notification popup has not been tested with this version.
3. The border-radius on the forward button needs to be removed.
Attachment #553763 - Attachment is obsolete: true
Attachment #554797 - Attachment is obsolete: true
Attachment #555199 - Attachment is obsolete: true
Attachment #555212 - Attachment is obsolete: true
Attachment #555199 - Flags: feedback?(fryn)
Jared, while this no longer applies to the current patch, for future reference, I think `toolbar.mode` can be used instead of `toolbar.getAttribute("mode")`. https://developer.mozilla.org/en/XUL/Attribute/toolbar.mode
(In reply to Frank Yan [:fryn] from comment #40)
> Jared, while this no longer applies to the current patch, for future
> reference, I think `toolbar.mode` can be used instead of
> `toolbar.getAttribute("mode")`.
> https://developer.mozilla.org/en/XUL/Attribute/toolbar.mode

There's no "mode" property.
(In reply to Dão Gottwald [:dao] from comment #41)
> > https://developer.mozilla.org/en/XUL/Attribute/toolbar.mode
> There's no "mode" property.

Indeed there isn't. I was mistaken. What a confusing title. The path implies that it's an attribute, but the article title implies otherwise. Likewise for <https://developer.mozilla.org/en/XUL/Attribute/resizer.dir>. There ought to be a better notation...
Attached patch WIP of patch for bug 674744 i2 (obsolete) — Splinter Review
Currently still needs better support for styling the themes for Leopard and Snow Leopard. There is also a small glitch with the padding of the identity box when the forward button becomes disabled (before it disappears).
Attachment #552664 - Attachment is obsolete: true
Attachment #558694 - Attachment is obsolete: true
Attached patch Patch for bug 674744 (obsolete) — Splinter Review
This has been pushed to the UX branch:
https://hg.mozilla.org/projects/ux/rev/32341b414d17
Attachment #559618 - Attachment is obsolete: true
Attachment #559640 - Flags: feedback?(shorlander)
Have forward button active
Click on the identity box
Expected: top-left and bottom-left corners of identity box should be square, fit the location bar connected to forward button
In fact: the identity box top-left and bottom-left corners is round.
Attached patch Patch for bug 674744 (obsolete) — Splinter Review
Stephen: Can you please send me the correct border colors for Snow Leopard and also test it out on Lion?

Dao: I have tried tweaking the values for the amount that the #urlbar should move, but I can't find a value that won't cause the other toolbaritems to shift. Can you help me out here?
Attachment #559640 - Attachment is obsolete: true
Attachment #559640 - Flags: feedback?(shorlander)
Attachment #561132 - Flags: feedback?(shorlander)
Attachment #561132 - Flags: feedback?(dao)
Comment on attachment 561643 [details]
Addon Installation notification is messed up

This is from bug 684450.
Attachment #561643 - Attachment is obsolete: true
Attachment #559699 - Attachment is obsolete: true
ThankYou and Apologies
> Dao: I have tried tweaking the values for the amount that the #urlbar should
> move, but I can't find a value that won't cause the other toolbaritems to
> shift. Can you help me out here?

I'm not sure what you mean by "cause the other toolbaritems to shift." Wrong values should only cause the url bar to overlap the back button wrongly.
(In reply to Dão Gottwald [:dao] from comment #50)
> > Dao: I have tried tweaking the values for the amount that the #urlbar should
> > move, but I can't find a value that won't cause the other toolbaritems to
> > shift. Can you help me out here?
> 
> I'm not sure what you mean by "cause the other toolbaritems to shift." Wrong
> values should only cause the url bar to overlap the back button wrongly.

|margin-left: -28px;| and |padding-left: 28px| on the urlbar-container fits the correct amount of space to draw the forward button without overlapping the back button.

However, if we set |margin-left: -28px;| on the urlbar, then the width of the urlbar-container seems to shrink a little and the search box moves over.
Attached patch Patch for bug 674744 v13 (obsolete) — Splinter Review
Unbitrotted and fixed two typos.
Attachment #561132 - Attachment is obsolete: true
Attachment #561132 - Flags: feedback?(shorlander)
Attachment #561132 - Flags: feedback?(dao)
Attachment #563625 - Flags: ui-review?(shorlander)
Attachment #563625 - Flags: review?(dao)
Attached image Screenshot of Patch v13 (obsolete) —
Something not right with the latest patch. Or my build is messed up :)
I'm not sure why there is a gap in the screenshot that was attached (https://bugzilla.mozilla.org/attachment.cgi?id=564188).

It looks like the forward navigation was possible but the opacity of the forward button wasn't updated to show the button? Could this be coming from bug 691218? Also, did you apply the patch for bug 682534 before applying this patch (674744 is dependent on 682534)?
Attachment #563625 - Attachment is obsolete: true
Attachment #563625 - Flags: review?(dao)
Attachment #564225 - Flags: ui-review?(shorlander)
Attachment #564225 - Flags: review?(dao)
(In reply to Jared Wein [:jwein] from comment #54)
> Also, did you apply the patch for bug 682534 before applying
> this patch (674744 is dependent on 682534)?

I did not. Thanks!
Comment on attachment 564225 [details] [diff] [review]
Patch for bug 674744 v13 (rebased)

Review of attachment 564225 [details] [diff] [review]:
-----------------------------------------------------------------

A few things I noticed:
- The mask is not properly sized(aligned?) so the background is bleeding through the back-button
- Hovering over the back-button causes the forward button to reappear behind the location-bar
- The forward-button has a dark drop-shadow, it should match the white etch from the location-bar
- The animation still causes the location-bar+search-field to shift on a new profile unless you manually resize it first

I will attach a screenshot.
Attachment #564225 - Flags: ui-review?(shorlander) → ui-review-
Attached patch Patch for bug 674744 v14 (obsolete) — Splinter Review
This patch fixes the mask issues and also works now with Personas.

There are two issues still with the shifting of the toolbar items and the forward button bleeding through.

The "forward button bleeding through" issue can also be seen in bug 682534.
Attachment #564225 - Attachment is obsolete: true
Attachment #564225 - Flags: review?(dao)
Attached patch Patch for bug 674744 v15 (obsolete) — Splinter Review
Updated the patch to use a constant for the width of the forward button.
Attachment #565071 - Attachment is obsolete: true
Attachment #566405 - Flags: review?(dolske)
Attachment #566405 - Flags: review?(dolske) → review?(dao)
Comment on attachment 566405 [details] [diff] [review]
Patch for bug 674744 v15

>+@conditionalForwardWithUrlbar@ + #urlbar-container > #urlbar {
>+  position: relative;

Is this needed? The winstripe patch didn't have this.

Half of this patch's file size is from toolbar-noise-lion.png, which seems unexpectedly large. Can this file be optimized? Could you get rid of it entirely by making the button texture transparent?
Attached patch Patch for bug 674744 v16 (obsolete) — Splinter Review
The |position: relative;| was unneeded.

We could probably make the texture smaller so the file size isn't as large.I'm not sure how we would want to change the color values of the linear gradients if we made the back button transparent.
Attachment #566405 - Attachment is obsolete: true
Attachment #566405 - Flags: review?(dao)
Attachment #566633 - Flags: review?(dao)
(In reply to Jared Wein [:jwein] from comment #61)
> Created attachment 566633 [details] [diff] [review] [diff] [details] [review]
> Patch for bug 674744 v16
> 
> The |position: relative;| was unneeded.
> 
> We could probably make the texture smaller so the file size isn't as
> large.I'm not sure how we would want to change the color values of the
> linear gradients if we made the back button transparent.

I have styles that would work to make it translucent. Does it ever overlap anything that isn't the toolbar background?
(In reply to Stephen Horlander from comment #62)
> (In reply to Jared Wein [:jwein] from comment #61)
> > Created attachment 566633 [details] [diff] [review] [diff] [details] [review] [diff] [details] [review]
> > Patch for bug 674744 v16
> > 
> > The |position: relative;| was unneeded.
> > 
> > We could probably make the texture smaller so the file size isn't as
> > large.I'm not sure how we would want to change the color values of the
> > linear gradients if we made the back button transparent.
> 
> I have styles that would work to make it translucent. Does it ever overlap
> anything that isn't the toolbar background?

Not that I can think of.
(In reply to Dão Gottwald [:dao] from comment #60)
> Half of this patch's file size is from toolbar-noise-lion.png, which seems
> unexpectedly large. Can this file be optimized? Could you get rid of it
> entirely by making the button texture transparent?

Jared, if this PNG is coming from Photoshop, just run optipng on it. Example instructions to remove unnecessary layers + alpha:

Remove gamma and color profiles:
  pngcrush -rem gAMA -rem cHRM -rem iCCP -rem sRGB

Optimize file size:
  optipng -o7
Comment on attachment 566633 [details] [diff] [review]
Patch for bug 674744 v16

>+      <svg:rect x="0" y="-5" width="1000000" height="55" fill="white"/>

nit: use width="10000" like I did for winstripe.

Cancelling review request as per previous comments.
Attachment #566633 - Flags: review?(dao)
Attached patch Patch for bug 674744 v17 (obsolete) — Splinter Review
Running pngcrush and optipng shrunk the file size by 22 bytes (probably not as much as hoped). I have also fixed the width to match the winstripe implementation.
Attachment #566633 - Attachment is obsolete: true
Attachment #567795 - Flags: review?(dao)
Comment on attachment 567795 [details] [diff] [review]
Patch for bug 674744 v17

As I understand it, we don't need the image at all.
Attachment #567795 - Flags: review?(dao)
(In reply to Dão Gottwald [:dao] from comment #67)
> Comment on attachment 567795 [details] [diff] [review] [diff] [details] [review]
> Patch for bug 674744 v17
> 
> As I understand it, we don't need the image at all.

Yeah I think we can probably get away with just using translucent styles if it doesn't overlap anything else.

I will try it and see.
Depends on: 694084
Attached patch Patch for bug 674744 v18 (obsolete) — Splinter Review
I have updated the CSS to use new styles provided by shorlander that remove the need for the toolbar-noise-lion.png.
Attachment #567795 - Attachment is obsolete: true
Attachment #572168 - Flags: ui-review?(shorlander)
Attachment #572168 - Flags: review?(dao)
Comment on attachment 572168 [details] [diff] [review]
Patch for bug 674744 v18

>+@conditionalForwardWithUrlbar@ > #forward-button:not(:-moz-lwtheme) {
>+  -moz-appearance: none;
>+  -moz-padding-start: 2px;
>+  background: -moz-linear-gradient(hsl(0,0%,99%), hsl(0,0%,67%));
>+  background-clip: padding-box;

nit:
background: -moz-linear-gradient(hsl(0,0%,99%), hsl(0,0%,67%)) padding-box;

>+@conditionalForwardWithUrlbar@ > #forward-button:hover:active:not(:-moz-lwtheme) {
>+  background: -moz-linear-gradient(hsl(0,0%,74%), hsl(0,0%,61%));
>+  background-clip: padding-box;

background-image: -moz-linear-gradient(hsl(0,0%,74%), hsl(0,0%,61%));

>+@conditionalForwardWithUrlbar@ > #forward-button:-moz-window-inactive:not(:-moz-lwtheme) {
>+  border-color: hsl(0,0%,64%) hsl(0,0%,65%) hsl(0,0%,66%);
>+  background: -moz-linear-gradient(hsl(0,0%,99%), hsl(0,0%,82%));

Are you sure you want to reset background-clip here?

>+@conditionalForwardWithUrlbar@ #identity-box:-moz-locale-dir(ltr),
>+@conditionalForwardWithUrlbar@ #identity-box:-moz-locale-dir(rtl) {
>+  border-radius: 0;
>+}

This selector won't match the identity box.
Attachment #572168 - Flags: review?(dao) → review-
Thanks for the quick review! I've addressed your review comments in this new version.
Attachment #572168 - Attachment is obsolete: true
Attachment #572168 - Flags: ui-review?(shorlander)
Attachment #572228 - Flags: ui-review?(shorlander)
Attachment #572228 - Flags: review?(dao)
Attachment #572228 - Flags: review?(dao) → review+
There is an issue with the (hidden) forward button bleeding through on hover of the back button. I've filed bug 700363 as a follow-up bug.

Pushed to fx-team: https://hg.mozilla.org/integration/fx-team/rev/e27e65738126
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/e27e65738126
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 10
Is it expected that this behavior is only present in fresh profiles but not existing ones?
Not sure if this should be a follow up bug, but can the conditional forward button be implemented in a way that adapts to the urlbar height?

That would be more future proof and would also fix the problem with add-ons that put icons in the urlbar, like Read It Later and FlagFox. Some of those icons increase the urlbar height, which in turn makes the conditional forward button look ugly: http://cl.ly/CT1s
(In reply to Henrik Skupin (:whimboo) from comment #74)
> Is it expected that this behavior is only present in fresh profiles but not
> existing ones?

This has been caused by the home button which was located between the back/forward buttons and the location bar. Moving it to the default location at the end of the navbar fixes it. I still wonder how many people will not be able to see this new feature because they are using older profiles with the home button left of the location bar.
(In reply to Reuben Morais [:reuben] from comment #75)
> Not sure if this should be a follow up bug, but can the conditional forward
> button be implemented in a way that adapts to the urlbar height?
> 
> That would be more future proof and would also fix the problem with add-ons
> that put icons in the urlbar, like Read It Later and FlagFox. Some of those
> icons increase the urlbar height, which in turn makes the conditional
> forward button look ugly: http://cl.ly/CT1s

Yeah, that looks really ugly. Although I think the fix for that is forcing the urlbar height, because if the forward button grows in size it will look equally ugly. Can you please see if a bug has been filed for this or file a bug if not?
This is a bug in the Read It Later and FlagFox add-ons, not in our code.
(In reply to Dão Gottwald [:dao] from comment #78)
> This is a bug in the Read It Later and FlagFox add-ons, not in our code.

Yeah I agree, but I think there is a bug in our code such that we allow an add-on to inadvertently increase the height of the urlbar. Most users will not associate an add-on with the reason the urlbar + forward button look ugly.
(In reply to Dão Gottwald [:dao] from comment #78)
> This is a bug in the Read It Later and FlagFox add-ons, not in our code.

All they do is insert a 16x16px image in the urlbar with a XUL overlay, and the problem only happens after the bookmark icon is shown. I've filed bug 709435 with more details.
Depends on: 714170
No longer blocks: 700363
Depends on: 700363
You need to log in before you can comment on or make changes to this bug.