Closed
Bug 1093820
Opened 10 years ago
Closed 9 years ago
DevEdition doesn't draw tabs in the titlebar on Windows
Categories
(DevTools :: General, defect)
Tracking
(firefox38 fixed)
RESOLVED
FIXED
Firefox 38
Tracking | Status | |
---|---|---|
firefox38 | --- | fixed |
People
(Reporter: canuckistani, Assigned: bgrins)
References
(Blocks 1 open bug)
Details
(Whiteboard: [devedition-polish])
Attachments
(10 files, 4 obsolete files)
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
Gijs
:
review+
|
Details | Diff | Splinter Review |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details |
We have an ugly title bar issue on Windows that is worse even than lightweight themes. This: http://screencast.com/t/3ejpMW29M Should look more like this: http://screencast.com/t/wb343LgoE ( eg the black should extend all the way to the top )
Reporter | ||
Updated•10 years ago
|
Group: mozilla-employee-confidential
Reporter | ||
Updated•10 years ago
|
Blocks: fx-dev-edition
Assignee | ||
Comment 1•10 years ago
|
||
(In reply to Jeff Griffiths (:canuckistani) from comment #0) > We have an ugly title bar issue on Windows that is worse even than > lightweight themes. This: > > http://screencast.com/t/3ejpMW29M > > Should look more like this: > > http://screencast.com/t/wb343LgoE > > ( eg the black should extend all the way to the top ) This is by design, right Stephen? I was working off of http://cl.ly/image/3x3Y0616103o/o. The problem is if you have the black extend all the way to the top there is a weird issue where the blue windows application border on the sides doesn't continue onto the top. You can see it in the dark lw theme screenshot you posted, but it's way more obvious and looks more off with the solid dark bg. I think there are some other plans for Windows with the tab strip in the future but I believe they don't include making the dark extend all the way to the top. This should probably be closed as a dup of 1091012, but I'll wait for his feedback.
Flags: needinfo?(shorlander)
Reporter | ||
Comment 2•10 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #1) ... > This is by design, right Stephen? I was working off of > http://cl.ly/image/3x3Y0616103o/o. If that is the expected effect, so be it. The feedback we've gotten is that it is jarring, and that people prefer the way lightweight themes handle the titlebar. Curious to hear shorlander's reaction to feedback though. Aside: I feel stupid for not having reviewed mockups for all platforms, as this caught me by surprise.
Comment 3•10 years ago
|
||
Because of platform limitations (bug 590945) LWT on Windows are pretty broken looking. They are especially broken on Windows 8 because the caption button cutout is from Windows 7 and just looks wrong with Windows 8's flat aesthetic. Not to mention alignment issues, layering issues and styling problems. This isn't ideal state even for LWTs. But being a secondary feature has given it a low priority for improvements. For something we are shipping by default I would rather not highlight the visually broken parts. Instead it was an intentional attempt to separate our chrome from the window like layers not something that is sort of melded but not really (LTWs) http://cl.ly/image/0q2d063Y3n1F As discussed elsewhere I think we can improve the way this looks and reduce our UI footprint further with something more like this: http://cl.ly/image/3Q3m1m122E0y/o For comparison you would be looking at something like this with the current LWT approach: http://cl.ly/image/2T11240M3p08
Flags: needinfo?(shorlander)
Assignee | ||
Comment 4•10 years ago
|
||
Renaming this to match Bug 1094464's title, but keeping this one open since it has more context
Summary: Dark theme doesn't look as good as lightweight dark themes. → DevEdition doesn't draw tabs in the titlebar on Windows
Comment 6•10 years ago
|
||
(In reply to Stephen Horlander [:shorlander] from comment #3) > As discussed elsewhere I think we can improve the way this looks and reduce > our UI footprint further with something more like this: > http://cl.ly/image/3Q3m1m122E0y/o We can do that by just removing the background color from the #TabsToolbar. Brian, is this something that we can get fixed before release (I ask this not knowing what limitations/deadlines we are up against)?
Flags: needinfo?(bgrinstead)
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6) > (In reply to Stephen Horlander [:shorlander] from comment #3) > > As discussed elsewhere I think we can improve the way this looks and reduce > > our UI footprint further with something more like this: > > http://cl.ly/image/3Q3m1m122E0y/o > > We can do that by just removing the background color from the #TabsToolbar. > Brian, is this something that we can get fixed before release (I ask this > not knowing what limitations/deadlines we are up against)? There is a weird gap between the tabs and the overflow controls if we do that. See this screenshot - https://www.dropbox.com/s/9lu4zllp0dc0qsd/Screenshot%202014-11-06%2010.57.19.png?dl=0. I'm sure that this is fixable, but it will take me a while since I need get context for how it is working and make sure that any change I make doesn't break something else. I'm all for updating it this way in Windows, but I'm not going to have time to do it and test it before tomorrow afternoon (when we are doing the merge). Jared, any chance you'd be able to do it?
Flags: needinfo?(bgrinstead) → needinfo?(jaws)
Comment 8•10 years ago
|
||
We can set the chromemargin attribute on #main-window to 0 to get rid of the aero border, but I'm guessing you'd like to avoid changing the window DOM for this.
Comment 9•10 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #7) > There is a weird gap between the tabs and the overflow controls if we do > that. See this screenshot - > https://www.dropbox.com/s/9lu4zllp0dc0qsd/Screenshot%202014-11-06%2010.57.19. > png?dl=0. If I had to guess I would look at tabCurveHalfWidth e.g. https://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/tabs.inc.css#110
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to Matthew N. [:MattN] (focused on Loop) from comment #9) > (In reply to Brian Grinstead [:bgrins] from comment #7) > > There is a weird gap between the tabs and the overflow controls if we do > > that. See this screenshot - > > https://www.dropbox.com/s/9lu4zllp0dc0qsd/Screenshot%202014-11-06%2010.57.19. > > png?dl=0. > > If I had to guess I would look at tabCurveHalfWidth e.g. > https://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/tabs. > inc.css#110 That seems to be covered already by: http://dxr.mozilla.org/mozilla-central/source/browser/themes/shared/devedition.inc.css#74
Assignee | ||
Comment 11•10 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #10) > (In reply to Matthew N. [:MattN] (focused on Loop) from comment #9) > > (In reply to Brian Grinstead [:bgrins] from comment #7) > > > There is a weird gap between the tabs and the overflow controls if we do > > > that. See this screenshot - > > > https://www.dropbox.com/s/9lu4zllp0dc0qsd/Screenshot%202014-11-06%2010.57.19. > > > png?dl=0. > > > > If I had to guess I would look at tabCurveHalfWidth e.g. > > https://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/tabs. > > inc.css#110 > > That seems to be covered already by: > http://dxr.mozilla.org/mozilla-central/source/browser/themes/shared/ > devedition.inc.css#74 Oh wait, I see the other references: http://dxr.mozilla.org/mozilla-central/search?q=tabcurvehalfwidth&case=false. Maybe that will fix it..
Comment 12•10 years ago
|
||
I was hoping that define had already been converted to a variable so you could just set it to 0.
Assignee | ||
Comment 13•10 years ago
|
||
(In reply to Matthew N. [:MattN] (focused on Loop) from comment #12) > I was hoping that define had already been converted to a variable so you > could just set it to 0. That would be easiest, but I'm a little worried about perf after bug 1088771
Assignee | ||
Comment 14•10 years ago
|
||
I can take a look at this
Status: NEW → ASSIGNED
Flags: needinfo?(jaws)
Updated•10 years ago
|
No longer blocks: fx-dev-edition
Assignee | ||
Comment 15•10 years ago
|
||
This changes a couple of things on Windows to make it match shorlander's mockup from Comment 3. 1) Simplifies some of the window spacing things since the dark line doesn't intersect with the window controls anymore 2) Uses a solid hover color instead of partially transparent since it's actually now lined up on the window titlebar color (which is actually darker than the previous alpha one to help address concerns there) 3) Set a background color on the new tab button, which should be safe in shared/ since the other OSes have this overlaid on the same background color anyway. 3a) Use less opacity than default for the scroll arrow disabled state since there is so much contrast btw default toolbar coloring and the dark button color. 5) Transparent bg in windows for TabsToolbar and tabbrowser-tabs. I've tested in Windows 7 classic, Windows 7 basic, and Windows 7 aero. I've also done some testing with it applied on OSX and it seems fine.
Assignee: nobody → bgrinstead
Attachment #8518379 -
Flags: review?(MattN+bmo)
Assignee | ||
Comment 16•10 years ago
|
||
Assignee | ||
Comment 17•10 years ago
|
||
Assignee | ||
Comment 18•10 years ago
|
||
Assignee | ||
Comment 19•10 years ago
|
||
I really need some UX help here. Here are the options I see: 1) The current patch makes it look very similar to the mockup in the default case as you can see in the above screenshots. Unfortunately if you put a button in between the new tab button and the tabs list, then the tab stays black but gets moved off to the side: https://imgur.com/49hOC3F. We could make that background transparent: https://imgur.com/OhCNP0Q. On top of that, we need to not invert the icons to the right (easy), but then on hover we would want them inverted again if the hover state was dark (harder). We could also just use the default hover state for these detached buttons. Matt may find other cases where the current approach doesn't work perfectly. 2) Here is what it looks like with the full black bg including titlebar, which I think is what this bug report is asking for: https://imgur.com/jmUOcF4. Notice the problems mentioned in comment 3. 3) Here is what it would look like with a full black bg including titlebar, but with a bit of margin to make it look slightly less broken: https://imgur.com/0DMHIi2. 4) Or we could just leave it as is. But I don't know what the best option is, given the time frame involved (needs to land in < 24 hours)
Flags: needinfo?(shorlander)
Flags: needinfo?(madhava)
Comment 20•10 years ago
|
||
Updated•10 years ago
|
Attachment #8518479 -
Attachment description: Windows 8 → Windows 10
Assignee | ||
Comment 21•10 years ago
|
||
Comment on attachment 8518379 [details] [diff] [review] windows-fixes.patch Clearing the review.. this gets weird when dealing with transparent buttons on the tab bar, since they need to be non-inverted while the add tab button would need to be inverted. Since the inversion state is controlled by [brighttext] on the toolbar I think it's going to very hairy to deal with. I'd say for what we could realistically expect to do tomorrow, it'd be options 2-4 in Comment 19.
Attachment #8518379 -
Flags: review?(MattN+bmo)
Comment 22•10 years ago
|
||
IMO, Option 3 looks the best. Let's push forward with that. Thanks for getting at this so quickly!
Comment 23•10 years ago
|
||
As talked about on IRC, I would prefer option 1, but if option 3 is the best option for the timeframe then go for that.
Flags: needinfo?(shorlander)
Flags: needinfo?(madhava)
Assignee | ||
Comment 24•10 years ago
|
||
There wasn't any time to get this implemented and tested before merging. It may be better to revisit this and come up with a solid plan for it while under less time pressure anyway.
Assignee | ||
Updated•10 years ago
|
Group: mozilla-employee-confidential
Assignee | ||
Updated•10 years ago
|
Whiteboard: [devedition-polish]
Assignee | ||
Comment 25•10 years ago
|
||
This makes the tabs looks like they do in the attached screenshots (option 1, shorlander's preference)
Attachment #8518379 -
Attachment is obsolete: true
Attachment #8524090 -
Flags: review?(gijskruitbosch+bugs)
Comment 26•10 years ago
|
||
(In reply to Stephen Horlander [:shorlander] from comment #3) > Because of platform limitations (bug 590945) LWT on Windows are pretty > broken looking. They are especially broken on Windows 8 because the caption > button cutout is from Windows 7 and just looks wrong with Windows 8's flat > aesthetic. Not to mention alignment issues, layering issues and styling > problems. Naively speaking though, I'm fairly sure we should be able to fix the cutout thing on Windows 8 separately - can you file a bug for this?
Flags: needinfo?(shorlander)
Comment 27•10 years ago
|
||
Comment on attachment 8524090 [details] [diff] [review] windows-titlebar.patch Review of attachment 8524090 [details] [diff] [review]: ----------------------------------------------------------------- This looks OK on the dark theme, but the light grey looks odd on the light theme with a colorful windows 8 titlebar background. Can we just fall back to transparent and default colors in that case? Don't know how hard that is and if shorlander is on board with that... (good thing I set a needinfo already!) Ditto for the new tab button. ::: browser/themes/shared/devedition.inc.css @@ +264,5 @@ > } > > .tabbrowser-arrowscrollbox > .scrollbutton-down, > .tabbrowser-arrowscrollbox > .scrollbutton-up { > + background-image: none; Should this be in shared? I guess it kind of makes sense in case other things use gradients that we don't want in the devedition theme, but even so... maybe be conservative and put this in the Windows only file? @@ +304,5 @@ > } > > /* Don't need space for the tab curves (66px - 30px) */ > .tabs-newtab-button { > + background-color: var(--tab-background-color); Here, too, not super sure this should be in shared...
Attachment #8524090 -
Flags: review?(gijskruitbosch+bugs)
Comment 28•10 years ago
|
||
Also, I don't see a separator between the last tab and the new tab button if it's adjacent to the tabs and they are not overflowing.
Assignee | ||
Comment 30•10 years ago
|
||
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #29) > Any update on this Brian? Still waiting on UX help from comment 27 - I talked with Stephen yesterday, who said he would take a look at it.
Flags: needinfo?(bgrinstead)
Assignee | ||
Comment 31•10 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #27) > Comment on attachment 8524090 [details] [diff] [review] > windows-titlebar.patch > > Review of attachment 8524090 [details] [diff] [review]: > ----------------------------------------------------------------- > > This looks OK on the dark theme, but the light grey looks odd on the light > theme with a colorful windows 8 titlebar background. Can we just fall back > to transparent and default colors in that case? Don't know how hard that is > and if shorlander is on board with that... (good thing I set a needinfo > already!) Ditto for the new tab button. I'm not clear about what you mean here - what would be transparent, and what would be the default colors?
Flags: needinfo?(gijskruitbosch+bugs)
Comment 32•10 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #31) > (In reply to :Gijs Kruitbosch from comment #27) > > Comment on attachment 8524090 [details] [diff] [review] > > windows-titlebar.patch > > > > Review of attachment 8524090 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > This looks OK on the dark theme, but the light grey looks odd on the light > > theme with a colorful windows 8 titlebar background. Can we just fall back > > to transparent and default colors in that case? Don't know how hard that is > > and if shorlander is on board with that... (good thing I set a needinfo > > already!) Ditto for the new tab button. > > I'm not clear about what you mean here - what would be transparent, and what > would be the default colors? IMO for the light theme, background tabs should be transparent so the Windows titlebar color is visible as their background, rather than the light grey. Same for the new tab button.
Flags: needinfo?(gijskruitbosch+bugs)
Comment 33•10 years ago
|
||
Actually, seeing this mockup [0], I think it looks fine (scroll down for light theme). I think it'll look odd if we remove the light gray bg, because the selected tab color will not play well with some titlebar colors. [0] : http://cl.ly/image/3Q3m1m122E0y/o
Assignee | ||
Comment 34•10 years ago
|
||
> > (In reply to :Gijs Kruitbosch from comment #27)
> IMO for the light theme, background tabs should be transparent so the
> Windows titlebar color is visible as their background, rather than the light
> grey. Same for the new tab button.
Like Tim, I think the mockup with the light theme looks nice - are you suggesting that the design should change or that it's not looking like that for you with this patch applied?
Assignee | ||
Comment 35•10 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #27) > Comment on attachment 8524090 [details] [diff] [review] > windows-titlebar.patch > > Review of attachment 8524090 [details] [diff] [review]: > ----------------------------------------------------------------- > > This looks OK on the dark theme, but the light grey looks odd on the light > theme with a colorful windows 8 titlebar background. Can we just fall back > to transparent and default colors in that case? Don't know how hard that is > and if shorlander is on board with that... (good thing I set a needinfo > already!) Ditto for the new tab button. > > ::: browser/themes/shared/devedition.inc.css > @@ +264,5 @@ > > } > > > > .tabbrowser-arrowscrollbox > .scrollbutton-down, > > .tabbrowser-arrowscrollbox > .scrollbutton-up { > > + background-image: none; > > Should this be in shared? I guess it kind of makes sense in case other > things use gradients that we don't want in the devedition theme, but even > so... maybe be conservative and put this in the Windows only file? > Sure, I've moved the background-image: none to the windows file > @@ +304,5 @@ > > } > > > > /* Don't need space for the tab curves (66px - 30px) */ > > .tabs-newtab-button { > > + background-color: var(--tab-background-color); > > Here, too, not super sure this should be in shared... Also moved. (In reply to :Gijs Kruitbosch from comment #28) > Also, I don't see a separator between the last tab and the new tab button if > it's adjacent to the tabs and they are not overflowing. Erg, that looks like the 2px change in bug 1096542 plus the new background on the new tab button is causing this. I wish we could set that to 0 without causing the tab shaking problem because it also causes a pretty obvious visual issue in windows where the tabs don't start for 2px (bug 1100578).
Comment 36•10 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #34) > > > (In reply to :Gijs Kruitbosch from comment #27) > > IMO for the light theme, background tabs should be transparent so the > > Windows titlebar color is visible as their background, rather than the light > > grey. Same for the new tab button. > > Like Tim, I think the mockup with the light theme looks nice - are you > suggesting that the design should change or that it's not looking like that > for you with this patch applied? I'm going from memory as my tree is busy with other things right now, but I believe that is roughly how it looks, so I guess I take issue with the design. Try it with an orange/yellow Windows titlebar, no pinned tabs, and the selected tab at the end.
Comment 37•10 years ago
|
||
This is what the current patch looks like on my win8 machine. Note in particular that the blue highlight on the selected tab connects weirdly to the un-selected tabs, and IMO the top border of the navbar here, combined with the 1-2px dead space inbetween the urlbar and that border, and the light border at the top of the selected tab, the grey not matching the grey of the navbar itself, it all makes it seem sloppy and disconnected (4 different shades of white/grey/black in 5px of vertical space).
Comment 38•10 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #37) > Created attachment 8536605 [details] > light-theme-screenshot.png > > This is what the current patch looks like on my win8 machine. Note in > particular that the blue highlight on the selected tab connects weirdly to > the un-selected tabs, and IMO the top border of the navbar here, combined > with the 1-2px dead space inbetween the urlbar and that border, and the > light border at the top of the selected tab, the grey not matching the grey > of the navbar itself, it all makes it seem sloppy and disconnected (4 > different shades of white/grey/black in 5px of vertical space). The mockup shows a border around the gray background, maybe the fact that it's missing makes the tabs look bad.
Assignee | ||
Comment 39•9 years ago
|
||
Latest patch - includes a border around the side and top of the tabs as in the following mockup: http://cl.ly/image/3Q3m1m122E0y/o
Attachment #8524090 -
Attachment is obsolete: true
Assignee | ||
Comment 40•9 years ago
|
||
Assignee | ||
Comment 41•9 years ago
|
||
This adds the Windows border all the way around the tabs as in the mockups (in both overflowing and non-overflowing cases). Also, the separator between the last tab and new tab button is back if you apply the patch from bug 1121723.
Attachment #8550586 -
Attachment is obsolete: true
Attachment #8557383 -
Flags: review?(gijskruitbosch+bugs)
Comment 42•9 years ago
|
||
(I'll be reviewing this first thing on Monday, sorry for the delay.)
Comment 43•9 years ago
|
||
Comment on attachment 8557383 [details] [diff] [review] windows-tabs-in-titlebar.patch Review of attachment 8557383 [details] [diff] [review]: ----------------------------------------------------------------- This looks better to me, but the left border is missing when in customize mode, and there is no right border to pinned tabs when the tabs are overflown. ::: browser/themes/windows/devedition.css @@ +58,5 @@ > +#main-window[sizemode=normal]:not([customizing]) .tabbrowser-tabs[overflow="true"] { > + background-clip: padding-box; > + border-top: 1px solid var(--chrome-nav-bar-separator-color); > + -moz-border-end: 1px solid var(--chrome-nav-bar-separator-color); > + background-color: var(--tab-background-color); /* Make sure there is no transparent gap during tab close animation */ This is interesting, because I still see this when not overflowing... I don't suppose there's anything we can do about that?
Attachment #8557383 -
Flags: review?(gijskruitbosch+bugs) → feedback+
Assignee | ||
Comment 44•9 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #43) > This is interesting, because I still see this when not overflowing... I > don't suppose there's anything we can do about that? I can't find any way to deal with this. There is no element in the background that collapses to the current width of the scrollbox contents (everything takes up the full width of the tab strip. Matt and I discussed adding an element to the scrollbox for this but there is a chance that it could break compatibility with addons, especially since it would require a change to all scrollboxes.
Assignee | ||
Comment 45•9 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #43) > Comment on attachment 8557383 [details] [diff] [review] > windows-tabs-in-titlebar.patch > > Review of attachment 8557383 [details] [diff] [review]: > ----------------------------------------------------------------- > > This looks better to me, but the left border is missing when in customize > mode I see that, we are removing the border in the main devedition.inc file, but I've checked and that rule is no longer necessary (it looks better with the border now on all platforms). > and there is no right border to pinned tabs when the tabs are > overflown. It looks like you mean no right border only on the rightmost pinned tab? I notice that in the default theme there isn't either (it looks like there is instead a left border on the first non-pinned tab after the scroll arrow). This behavior seems to change due to my patch in Bug 1121723, so I will take a look there.
Assignee | ||
Comment 46•9 years ago
|
||
This fixes the customize mode UI, and if you apply the latest patch in Bug 1121723 there will be a separator between the arrow scrollbox and the first unpinned tab (as in the default theme).
Attachment #8557383 -
Attachment is obsolete: true
Attachment #8558668 -
Flags: review?(gijskruitbosch+bugs)
Comment 47•9 years ago
|
||
Comment on attachment 8558668 [details] [diff] [review] windows-tabs-in-titlebar.patch Review of attachment 8558668 [details] [diff] [review]: ----------------------------------------------------------------- I applied both this and bug 1121723 's last patch and bug 1128719 and this patch, and I still see no separator between the pinned tab and the overflow button (trying on top of a8bbf40c80b6 on fx-team on win8.1)... Am I missing something?
Attachment #8558668 -
Flags: review?(gijskruitbosch+bugs)
Assignee | ||
Comment 48•9 years ago
|
||
> I applied both this and bug 1121723 's last patch and bug 1128719 and this > patch, and I still see no separator between the pinned tab and the overflow > button (trying on top of a8bbf40c80b6 on fx-team on win8.1)... > > Am I missing something? As I said in Comment 45, the separator in the default theme is not between the last pinned tab and the overflow button, but rather between the overflow button and the first unpinned tab. So I just kept it the same, since I haven't seen any specific mockups for Dev Edition that show otherwise. Attaching a screenshot pointing to the separator in question to make sure we are talking about the same thing (screeenshot is on osx but I remember the same thing in Windows 7).
Comment 49•9 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #48) > Created attachment 8559190 [details] > tab-overflow-separator.png > > > I applied both this and bug 1121723 's last patch and bug 1128719 and this > > patch, and I still see no separator between the pinned tab and the overflow > > button (trying on top of a8bbf40c80b6 on fx-team on win8.1)... > > > > Am I missing something? > > As I said in Comment 45, the separator in the default theme is not between > the last pinned tab and the overflow button, but rather between the overflow > button and the first unpinned tab. So I just kept it the same, since I > haven't seen any specific mockups for Dev Edition that show otherwise. > Attaching a screenshot pointing to the separator in question to make sure we > are talking about the same thing (screeenshot is on osx but I remember the > same thing in Windows 7). Ah, I see it now. Yes, we're talking about the same thing. On Windows though, the difference is IMO the fact that the background of the tabs area is different and has a border, and so the lacking border between the pinned tab and the overflow button, esp. if there are other pinned tabs present, looks odd.
Assignee | ||
Comment 50•9 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #49) > Created attachment 8559752 [details] > pinned-tab-border.PNG > > Ah, I see it now. Yes, we're talking about the same thing. On Windows > though, the difference is IMO the fact that the background of the tabs area > is different and has a border, and so the lacking border between the pinned > tab and the overflow button, esp. if there are other pinned tabs present, > looks odd. I do see what you are saying, but I'd like to still proceed with the patch if it's OK with you and file a follow up bug to request UX help for this case. Because it's an existing issue (across all platforms) and this patch improves lots of other things on Windows, so I wouldn't want to hold up those changes during whole process of designing an implementing that change.
Comment 51•9 years ago
|
||
Comment on attachment 8558668 [details] [diff] [review] windows-tabs-in-titlebar.patch Review of attachment 8558668 [details] [diff] [review]: ----------------------------------------------------------------- OK, let's go with this then.
Attachment #8558668 -
Flags: review+
Assignee | ||
Comment 52•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/b017c79e4ebc
Whiteboard: [devedition-polish] → [fixed-in-fx-team][devedition-polish]
Comment 53•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b017c79e4ebc
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox38:
--- → fixed
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team][devedition-polish] → [devedition-polish]
Target Milestone: --- → Firefox 38
Updated•9 years ago
|
QA Contact: cornel.ionce
Assignee | ||
Comment 54•9 years ago
|
||
Screenshots of the theme on win7 after this bug has been resolved (just leaving it here for reference).
Updated•7 years ago
|
Flags: needinfo?(shorlander)
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•