Closed
Bug 674744
Opened 13 years ago
Closed 13 years ago
Implement conditional forward button for pinstripe
Categories
(Firefox :: Theme, enhancement)
Tracking
()
RESOLVED
FIXED
Firefox 10
People
(Reporter: jaws, Assigned: jaws)
References
Details
(Whiteboard: [fixed-in-fx-team])
Attachments
(2 files, 26 obsolete files)
(deleted),
image/png
|
Details | |
(deleted),
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.
Assignee | ||
Comment 1•13 years ago
|
||
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
Comment 2•13 years ago
|
||
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.
Assignee | ||
Comment 3•13 years ago
|
||
(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?
Comment 4•13 years ago
|
||
It sounds like you want a second parallel transition that changes the urlbar's left padding or something like that.
Assignee | ||
Comment 5•13 years ago
|
||
(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.
Comment 6•13 years ago
|
||
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.
Assignee | ||
Comment 7•13 years ago
|
||
(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.
Assignee | ||
Comment 9•13 years ago
|
||
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
Assignee | ||
Comment 10•13 years ago
|
||
Fixed the jankiness when hovering over the back button if the forward button is disabled.
Attachment #550234 -
Attachment is obsolete: true
Assignee | ||
Comment 11•13 years ago
|
||
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
Assignee | ||
Comment 12•13 years ago
|
||
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•13 years ago
|
||
Is it going to be implemented for small icons mode too?
Comment 14•13 years ago
|
||
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•13 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•13 years ago
|
||
I second the question about small icons from comment 13.
Any plans on that, guys?
Assignee | ||
Comment 17•13 years ago
|
||
(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•13 years ago
|
||
(In reply to Jared Wein [:jwein] from comment #17)
Thanks Jared. Filed Bug 677860.
Assignee | ||
Comment 19•13 years ago
|
||
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•13 years ago
|
||
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.
Assignee | ||
Comment 21•13 years ago
|
||
(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.
Comment 22•13 years ago
|
||
When you set small icons and set big icons back the forward button is always shown.
Comment 23•13 years ago
|
||
Not sure if its known, but this has problems with personas on osx:
http://dl.dropbox.com/u/72157/screeny.png
Comment 24•13 years ago
|
||
popup windows lacking back button have shifted location bar problem
Comment 25•13 years ago
|
||
Screenshot of the problem pointed out in comment 24.
Assignee | ||
Comment 26•13 years ago
|
||
(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.
Assignee | ||
Updated•13 years ago
|
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
Assignee | ||
Comment 27•13 years ago
|
||
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)
Assignee | ||
Comment 28•13 years ago
|
||
Comment 29•13 years ago
|
||
Assignee | ||
Comment 30•13 years ago
|
||
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•13 years ago
|
||
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.
Comment 32•13 years ago
|
||
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.
Assignee | ||
Comment 33•13 years ago
|
||
(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 :)
Comment 34•13 years ago
|
||
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•13 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•13 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
Assignee | ||
Comment 37•13 years ago
|
||
(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.
Comment 38•13 years ago
|
||
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
Assignee | ||
Comment 39•13 years ago
|
||
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•13 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
Comment 41•13 years ago
|
||
(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•13 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...
Assignee | ||
Comment 43•13 years ago
|
||
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
Assignee | ||
Comment 44•13 years ago
|
||
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•13 years ago
|
||
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.
Assignee | ||
Comment 46•13 years ago
|
||
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•13 years ago
|
||
Comment 48•13 years ago
|
||
Comment on attachment 561643 [details]
Addon Installation notification is messed up
This is from bug 684450.
Attachment #561643 -
Attachment is obsolete: true
Updated•13 years ago
|
Attachment #559699 -
Attachment is obsolete: true
Comment 49•13 years ago
|
||
ThankYou and Apologies
Comment 50•13 years ago
|
||
> 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.
Assignee | ||
Comment 51•13 years ago
|
||
(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.
Assignee | ||
Comment 52•13 years ago
|
||
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)
Comment 53•13 years ago
|
||
Something not right with the latest patch. Or my build is messed up :)
Updated•13 years ago
|
Attachment #563625 -
Flags: ui-review?(shorlander)
Assignee | ||
Comment 54•13 years ago
|
||
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)
Comment 55•13 years ago
|
||
(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 56•13 years ago
|
||
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-
Comment 57•13 years ago
|
||
Attachment #564188 -
Attachment is obsolete: true
Assignee | ||
Comment 58•13 years ago
|
||
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)
Assignee | ||
Comment 59•13 years ago
|
||
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•13 years ago
|
Attachment #566405 -
Flags: review?(dolske) → review?(dao)
Comment 60•13 years ago
|
||
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?
Assignee | ||
Comment 61•13 years ago
|
||
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)
Comment 62•13 years ago
|
||
(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?
Assignee | ||
Comment 63•13 years ago
|
||
(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.
Comment 64•13 years ago
|
||
(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 65•13 years ago
|
||
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)
Assignee | ||
Comment 66•13 years ago
|
||
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 67•13 years ago
|
||
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)
Comment 68•13 years ago
|
||
(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.
Assignee | ||
Comment 69•13 years ago
|
||
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 70•13 years ago
|
||
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-
Assignee | ||
Comment 71•13 years ago
|
||
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•13 years ago
|
Attachment #572228 -
Flags: review?(dao) → review+
Updated•13 years ago
|
Attachment #572228 -
Flags: ui-review?(shorlander) → ui-review+
Assignee | ||
Comment 72•13 years ago
|
||
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]
Comment 73•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 10
Comment 74•13 years ago
|
||
Is it expected that this behavior is only present in fresh profiles but not existing ones?
Comment 75•13 years ago
|
||
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
Comment 76•13 years ago
|
||
(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.
Assignee | ||
Comment 77•13 years ago
|
||
(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?
Comment 78•13 years ago
|
||
This is a bug in the Read It Later and FlagFox add-ons, not in our code.
Assignee | ||
Comment 79•13 years ago
|
||
(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.
Comment 80•13 years ago
|
||
(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•13 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•