Implement conditional forward button for pinstripe

RESOLVED FIXED in Firefox 10

Status

()

Firefox
Theme
--
enhancement
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: jaws, Assigned: jaws)

Tracking

(Depends on: 1 bug)

Trunk
Firefox 10
All
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(2 attachments, 26 obsolete attachments)

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.
Created attachment 548943 [details] [diff] [review]
WIP for bug 674744

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 :)

Comment 8

6 years ago
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.
Created attachment 550234 [details] [diff] [review]
WIP for bug 674744

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
Created attachment 550278 [details] [diff] [review]
WIP for bug 674744 v3

Fixed the jankiness when hovering over the back button if the forward button is disabled.
Attachment #550234 - Attachment is obsolete: true
Created attachment 550313 [details] [diff] [review]
WIP for bug 674744 v4

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.

Comment 13

6 years ago
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

Comment 15

6 years ago
(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.

Comment 16

6 years ago
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.

Comment 18

6 years ago
(In reply to Jared Wein [:jwein] from comment #17)

Thanks Jared. Filed Bug 677860.
Created attachment 552574 [details] [diff] [review]
WIP for bug 674744 v5

Pushed to UX branch:
https://hg.mozilla.org/projects/ux/rev/83c065b68324

Pushed to try server:
http://tbpl.mozilla.org/?tree=Try&rev=384ae169ea1d
Attachment #550313 - Attachment is obsolete: true

Comment 20

6 years ago
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.
(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

Comment 24

6 years ago
popup windows lacking back button have shifted location bar problem

Comment 25

6 years ago
Created attachment 553763 [details]
Screenshot of shifted location bar in popups 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
Created attachment 554466 [details] [diff] [review]
WIP for bug 674744 v6

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)
Created attachment 554638 [details] [diff] [review]
WIP for bug 674744 v7
Created attachment 554797 [details]
Back button overlaps urlbar suggestions
Created attachment 555199 [details] [diff] [review]
WIP for bug 674744 v8

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)

Comment 31

6 years ago
Created attachment 555212 [details]
WIP v8 on Snow Leopard

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.

Comment 35

6 years ago
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.

Comment 36

6 years ago
(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
Created attachment 558694 [details] [diff] [review]
WIP of patch for bug 674744

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)

Comment 40

6 years ago
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.

Comment 42

6 years ago
(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...
Created attachment 559618 [details] [diff] [review]
WIP of patch for bug 674744 i2

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
Created attachment 559640 [details] [diff] [review]
Patch for bug 674744

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)

Comment 45

6 years ago
Created attachment 559699 [details]
When the forward button is actived, identity box shouldn't have boder radius (when coditional forward button enble)

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.
Created attachment 561132 [details] [diff] [review]
Patch for bug 674744

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 47

6 years ago
Created attachment 561643 [details]
Addon Installation notification is messed up
Comment on attachment 561643 [details]
Addon Installation notification is messed up

This is from bug 684450.
Attachment #561643 - Attachment is obsolete: true

Updated

6 years ago
Attachment #559699 - Attachment is obsolete: true

Comment 49

6 years ago
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.
Created attachment 563625 [details] [diff] [review]
Patch for bug 674744 v13

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)
Created attachment 564188 [details]
Screenshot of Patch v13

Something not right with the latest patch. Or my build is messed up :)
Attachment #563625 - Flags: ui-review?(shorlander)
Created attachment 564225 [details] [diff] [review]
Patch for bug 674744 v13 (rebased)

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-
Created attachment 564282 [details]
Screenshot of Patch v13
Attachment #564188 - Attachment is obsolete: true
Created attachment 565071 [details] [diff] [review]
Patch for bug 674744 v14

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)
Created attachment 566405 [details] [diff] [review]
Patch for bug 674744 v15

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)

Updated

6 years ago
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?
Created attachment 566633 [details] [diff] [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.
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)
Created attachment 567795 [details] [diff] [review]
Patch for bug 674744 v17

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.

Updated

6 years ago
Depends on: 694084
Created attachment 572168 [details] [diff] [review]
Patch for bug 674744 v18

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-
Created attachment 572228 [details] [diff] [review]
Patch for bug 674744 v18.1

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)

Updated

6 years ago
Attachment #572228 - Flags: review?(dao) → review+
Attachment #572228 - Flags: ui-review?(shorlander) → ui-review+
Blocks: 700363
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
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 10
Depends on: 703063
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.

Updated

6 years ago
Depends on: 714170

Updated

6 years ago
No longer blocks: 700363
Depends on: 700363
You need to log in before you can comment on or make changes to this bug.