Closed
Bug 658467
Opened 14 years ago
Closed 8 years ago
Fade out tab label on overflow instead of ellipsis
Categories
(Firefox :: Tabbed Browser, defect)
Firefox
Tabbed Browser
Tracking
()
VERIFIED
FIXED
Firefox 53
People
(Reporter: tdowner, Assigned: dao)
References
(Regressed 1 open bug)
Details
(Whiteboard: p=0 [qx:spec])
Attachments
(2 files, 11 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
image/png
|
Details |
Ala Chrome, using fadeout for text will probably give 1-2 more characters visible to the user, and just looks smoother.
I think this bug is directly related to the tab strip redesign. Assuming the tab will be chrome-like it should be better to have more space for the tab titles.
Comment 2•13 years ago
|
||
This would be a nice enhancement, but we will probably need some XUL platform changes to alter how crop works. If a new cropping mode was added to truncate instead of adding an ellipsis, then one idea is to use an SVG mask on the text. Unfortunately I'm not familiar enough with the tab code to know how complicated that would be.
Updated•13 years ago
|
Component: Theme → Tabbed Browser
QA Contact: theme → tabbed.browser
Updated•13 years ago
|
Component: Tabbed Browser → Theme
Comment 3•13 years ago
|
||
(In reply to Jared Wein [:jwein and :jaws] from comment #2)
> This would be a nice enhancement, but we will probably need some XUL
> platform changes to alter how crop works. If a new cropping mode was added
> to truncate instead of adding an ellipsis ...
I spoke too soon. There is a crop value for not adding an ellipsis, none ;)
If we just use text-overflow: clip; and overflow: hidden; and apply an SVG mask then I guess it would work.
Frank: Do you know of any reasons why we can't do that?
Comment 4•13 years ago
|
||
(In reply to Jared Wein [:jwein and :jaws] from comment #3)
> If we just use text-overflow: clip; and overflow: hidden; and apply an SVG
> mask then I guess it would work.
>
> Frank: Do you know of any reasons why we can't do that?
No. SVG is overly complex and slow, but it can get the job done. See my comments in bug 653670.
Interesting way to handle Tab title in Chrome : http://littlebigdetails.com/post/7577371131/chrome-the-title-text-only-shows-what
Comment 6•13 years ago
|
||
(In reply to Guillaume C. [:GE3K0S] from comment #5)
> Interesting way to handle Tab title in Chrome :
> http://littlebigdetails.com/post/7577371131/chrome-the-title-text-only-shows-
> what
Work on a similar feature is being handled in bug 583890.
Comment 8•13 years ago
|
||
(In reply to Guillaume C. [:GE3K0S] from comment #7)
> What the UX team is currently thinking about this change ?
We would like to use this method. As discussed in this bug we just need to figure out a viable method for implementation.
Could be interesting to use the same fade out in the bookmarks bar instead of ellipses.
Updated•12 years ago
|
Severity: enhancement → normal
Summary: On Tab Overflow fade out instead of ellipses → Fade out tab label on overflow instead of ellipsis
Updated•12 years ago
|
Blocks: australis-tabs
Updated•12 years ago
|
Whiteboard: [Australis:M3]
Comment 10•12 years ago
|
||
Perhaps something like http://www.webdesignerwall.com/demo/css-gradient-text/ would work?
Comment 11•12 years ago
|
||
(In reply to Mike de Boer [:mikedeboer] from comment #10)
> Perhaps something like
> http://www.webdesignerwall.com/demo/css-gradient-text/ would work?
This can't work because these images would have to be generated ahead of time and placed on top of the text. There would be a lot of images for each of our standard themes, and unlimited ones for lightweight themes.
Comment 12•12 years ago
|
||
So I looked at this a bit today. There are two problems I run into:
1) I don't see how to specify a pixel offset from the right side of the box for the gradient stops. In theory, I think maskUnits, maskContentUnits and gradientUnits should take care of this by setting them all to userSpaceOnUse. In practice, I haven't found a way of getting this right; it seems like we then actually use the width of the window as a reference for percentages (or, in the case of jsfiddle/jsbin, the <body>), so I can't easily make the gradient be the size of the tab label. I also can't make the width equal to the desired width of the fade (say, 20px or thereabouts) and then right align it, because SVG doesn't seem to support alignment stuff except on its view containers (where you can mess with the coordinate system inside). I don't know if using those with CSS masks is supported, and haven't tried. I'm sure I'm missing something obvious here...
This may be less important if we're OK with the degree of fade being a little different if tabs start being squashed.
2) if we switch to crop=none rather than crop=end as the default, the tabs resize to accommodate longer titles. Setting a max-width and/or display: block doesn't seem to help (and besides, we can't predict the size of the tabs, that should be determined by the tabs squashing), nor does text-overflow: clip and overflow: hidden as suggested in comment #3. I am not sure how to fix this. Ideas?
Comment 13•12 years ago
|
||
I will email folks and look at this some more the coming week.
Assignee: nobody → gijskruitbosch+bugs
Comment 14•12 years ago
|
||
I've emailed with dholbert and jwatt, but it seems this is Hard in SVG, more or less for the reasons cited in comment #12, point 1 - unless we don't care exactly if the fade is percentual rather than a fixed size (px/em), or if we're happy to adjust the fade size manually when we resize tabs... but that seems like a perf nightmare.
At the advice of our SVG folks, I've posted to the Yahoo svg-developers list, with my email currently in moderation.
Comment 15•12 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #14)
> I've emailed with dholbert and jwatt, but it seems this is Hard in SVG, more
> or less for the reasons cited in comment #12, point 1 - unless we don't care
> exactly if the fade is percentual rather than a fixed size (px/em), or if
> we're happy to adjust the fade size manually when we resize tabs... but that
> seems like a perf nightmare.
>
> At the advice of our SVG folks, I've posted to the Yahoo svg-developers
> list, with my email currently in moderation.
I can't think of a situation where we wouldn't want a fixed size fade.
Updated•12 years ago
|
Updated•12 years ago
|
Whiteboard: [Australis:M3] → [Australis:M6]
Comment 16•12 years ago
|
||
While I would like it, I don't see feasible solutions within the timeframe in which we want to ship Australis, and I don't think this should block Australis..
Comment 17•12 years ago
|
||
I agree it shouldn't block, but it would be nice to have some idea of how we plan to solve this in the (near) future.
Comment 18•12 years ago
|
||
(In reply to Frank Yan (:fryn) from comment #16)
> While I would like it, I don't see feasible solutions within the timeframe
> in which we want to ship Australis, and I don't think this should block
> Australis..
I was thinking the same thing as I changed the milestone.
Whiteboard: [Australis:M6]
Comment 19•12 years ago
|
||
PSA: Jonathan Watt posted about this issue in m.d.platform: https://groups.google.com/forum/#!topic/mozilla.dev.platform/-23mG3x2vdk
Updated•12 years ago
|
Whiteboard: [Australis:M-]
Updated•11 years ago
|
Assignee: gijskruitbosch+bugs → nobody
Updated•11 years ago
|
Whiteboard: [Australis:M-] → [Australis:M-][Australis:P?]
Comment 20•11 years ago
|
||
Removing from the Australis project. Still want, but not part of Australis.
No longer blocks: australis-tabs
Whiteboard: [Australis:M-][Australis:P?]
Updated•11 years ago
|
Blocks: fxdesktopbacklog
Whiteboard: [triage]
Updated•11 years ago
|
Whiteboard: [triage]
Updated•11 years ago
|
No longer blocks: fxdesktopbacklog
Updated•11 years ago
|
Blocks: fxdesktopbacklog
Whiteboard: [feature] p=0
Updated•11 years ago
|
Updated•10 years ago
|
Whiteboard: p=0 → p=0 [qx]
Updated•10 years ago
|
Whiteboard: p=0 [qx] → p=0 [qx:spec]
Comment 21•9 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #19)
> PSA: Jonathan Watt posted about this issue in m.d.platform:
> https://groups.google.com/forum/#!topic/mozilla.dev.platform/-23mG3x2vdk
The result of this thread seemed to be either a) implement and use http://www.w3.org/TR/css-masking/#the-mask-box-image or b) a spec extension to text-overflow
Jonathan do you know if there are plans to do either of these in Gecko?
Flags: needinfo?(jwatt)
Comment 22•9 years ago
|
||
(In reply to Stephen Horlander [:shorlander] from comment #21)
> The result of this thread seemed to be either a) implement and use
> http://www.w3.org/TR/css-masking/#the-mask-box-image
Bug 877294 hasn't seen much activity recently.
> or b) a spec extension to text-overflow
I just carried over the m.d.platform discussion to www-style:
https://lists.w3.org/Archives/Public/www-style/2015Aug/0087.html
We can see how that goes and file a bug to implement if it seems to fly.
Flags: needinfo?(jwatt)
Comment 23•9 years ago
|
||
Thanks!
Assignee | ||
Updated•9 years ago
|
Assignee | ||
Comment 24•9 years ago
|
||
This works with layout.css.background-clip-text.enabled = true
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Comment 25•9 years ago
|
||
\o/ So glad we're finally getting around to this.
Comment 26•9 years ago
|
||
Comment on attachment 8741814 [details] [diff] [review]
WIP patch
Review of attachment 8741814 [details] [diff] [review]:
-----------------------------------------------------------------
\o/
::: browser/base/content/tabbrowser.css
@@ +45,5 @@
> + background-image: linear-gradient(to right, transparent, currentColor 1em, currentColor);
> +}
> +
> +.tab-label {
> + -webkit-text-fill-color: transparent;
Do you need to use the webkit prefix? I thought you could just use `color`? As I understood this, `-webkit-text-fill-color` was only created to allow for graceful degradation in browsers that didn't support `background-clip: text;` so that the text color wouldn't be transparent in those browsers.
Assignee | ||
Comment 27•9 years ago
|
||
It's slightly more complicated (e.g. -webkit-text-fill-color doesn't change the computed color / currentColor), but in this case 'color' works as well.
Assignee | ||
Comment 28•9 years ago
|
||
Attachment #8741814 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
No longer depends on: mask-image
Comment 29•9 years ago
|
||
When background-clip:text pref is off
1. -webkit-text-fill-color:transparent - you can still see single color text.
2. color: transparent - no text at all.
Assignee | ||
Comment 30•9 years ago
|
||
Note that this disables sub-pixel AA. I don't think there's a way around that. Are we okay with that?
Attachment #8741946 -
Attachment is obsolete: true
Attachment #8742079 -
Flags: ui-review?(shorlander)
Assignee | ||
Comment 31•9 years ago
|
||
This applies the fade-out effect (and disables sub-pixel AA) only when the label overflows rather than all the time.
Attachment #8742079 -
Attachment is obsolete: true
Attachment #8742079 -
Flags: ui-review?(shorlander)
Attachment #8742097 -
Flags: ui-review?(shorlander)
Assignee | ||
Updated•9 years ago
|
Component: Theme → Tabbed Browser
Assignee | ||
Comment 32•9 years ago
|
||
I broke center-aligning the label on OS X. This should fix it.
Attachment #8742097 -
Attachment is obsolete: true
Attachment #8742097 -
Flags: ui-review?(shorlander)
Attachment #8742103 -
Flags: ui-review?(shorlander)
Assignee | ||
Comment 33•9 years ago
|
||
There appear to be a number of perf regressions:
https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=002880683dd4&newProject=try&newRevision=015e8fc318b2&framework=1
We could probably justify the tart and cart regressions as the price for a net UX win, but tp5, tresize and tsvgx are more troubling.
Comment 34•9 years ago
|
||
Comment on attachment 8742103 [details] [diff] [review]
patch v2
Review of attachment 8742103 [details] [diff] [review]:
-----------------------------------------------------------------
Unfortunately whatever this does to font rendering makes it look pretty bad in a bunch of different ways :-\
http://people.mozilla.org/~shorlander/drops/tab-label-fade-2016-04-28.png
Tested on OS X so far, will test elsewhere.
Attachment #8742103 -
Flags: ui-review?(shorlander) → ui-review-
Assignee | ||
Comment 35•9 years ago
|
||
(In reply to Stephen Horlander [:shorlander] from comment #34)
> Comment on attachment 8742103 [details] [diff] [review]
> patch v2
>
> Review of attachment 8742103 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> Unfortunately whatever this does to font rendering makes it look pretty bad
> in a bunch of different ways :-\
>
> http://people.mozilla.org/~shorlander/drops/tab-label-fade-2016-04-28.png
>
> Tested on OS X so far, will test elsewhere.
Have you seen similar issues elsewhere?
Flags: needinfo?(shorlander)
Comment 36•9 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #35)
> (In reply to Stephen Horlander [:shorlander] from comment #34)
> > Comment on attachment 8742103 [details] [diff] [review]
> > patch v2
> >
> > Review of attachment 8742103 [details] [diff] [review]:
> > -----------------------------------------------------------------
> >
> > Unfortunately whatever this does to font rendering makes it look pretty bad
> > in a bunch of different ways :-\
> >
> > http://people.mozilla.org/~shorlander/drops/tab-label-fade-2016-04-28.png
> >
> > Tested on OS X so far, will test elsewhere.
>
> Have you seen similar issues elsewhere?
I tested on Windows 7 and it had similar issues. They were not quite as pronounced. Though it did have one new issue that I couldn't reproduce where blocks of text would sometimes dissappear and reappear.
Flags: needinfo?(shorlander)
Comment 37•9 years ago
|
||
Will try and test on Linux and Windows 10, but I am having some build issues.
Assignee | ||
Comment 38•9 years ago
|
||
Already done builds that you can use:
http://archive.mozilla.org/pub/firefox/try-builds/dgottwald@mozilla.com-8ca011a856cf24c58a565525b1a8a35d3cd554eb/try-linux64/
http://archive.mozilla.org/pub/firefox/try-builds/dgottwald@mozilla.com-8ca011a856cf24c58a565525b1a8a35d3cd554eb/try-win32/
Flags: needinfo?(shorlander)
Comment 39•9 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #38)
> Already done builds that you can use:
>
> http://archive.mozilla.org/pub/firefox/try-builds/dgottwald@mozilla.com-
> 8ca011a856cf24c58a565525b1a8a35d3cd554eb/try-linux64/
>
> http://archive.mozilla.org/pub/firefox/try-builds/dgottwald@mozilla.com-
> 8ca011a856cf24c58a565525b1a8a35d3cd554eb/try-win32/
Thank you.
Comment 40•9 years ago
|
||
Problems that I encountered:
Windows 7 and 10:
- Slight font distortion: Smaller character width, overall thinner appearance (maybe due to sub-pixel AA loss?)
- Randomly disappearing and reappearing sections of text (attaching screenshot)
Windows XP:
- Significant font distortion: Smaller character width, overall thinner appearance AND some sections of text appearing more blurry than others
OS X:
- Significant font distortion: Smaller character width, overall thinner appearance AND some sections of text appearing more blurry than others
Linux:
- Looks great, no problems!
Unfortunately I don't think this approach will work with those bugs :(
Flags: needinfo?(shorlander)
Assignee | ||
Comment 41•9 years ago
|
||
Apparently background-clip:text has been re-implemented recently in bug 1269971. Maybe that solves the rendering glitches. New try builds: http://archive.mozilla.org/pub/firefox/try-builds/dgottwald@mozilla.com-e5018b0ecb05b5c79bd325d0bce03ffc8e3a2341/
Assignee | ||
Comment 42•9 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #41)
> Apparently background-clip:text has been re-implemented recently in bug
> 1269971.
Perf regressions seem to be less random and less catastrophic with that:
https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=8d3641032e29&newProject=try&newRevision=e5018b0ecb05&framework=1
Summary:
tabpaint opt
linux64 5.50%
tart opt
linux64 2.01%
windows7-32 4.23%
tart opt e10s
linux64 2.41%
windows7-32 3.28%
tps opt
windows7-32 2.53%
tps opt e10s
windows7-32 4.62%
Assuming we care less about non-e10s regressions, and assuming the tart regression is just a price we need to pay for a more sophisticated tab layout, that leaves the tps opt e10s regression as the main concern. Why that regression is only present on Windows, I have no idea :/
Assignee | ||
Comment 43•9 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #41)
> Apparently background-clip:text has been re-implemented recently in bug
> 1269971. Maybe that solves the rendering glitches. New try builds:
http://archive.mozilla.org/pub/firefox/try-builds/dgottwald@mozilla.com-
e5018b0ecb05b5c79bd325d0bce03ffc8e3a2341/
Stephen, could you please give these a try? I am seeing a weird bug on Windows 10 but it's different from what you saw previously.
Flags: needinfo?(shorlander)
Comment 44•9 years ago
|
||
This build addresses a lot of the problems I saw before.
On Windows 10, Windows 7 and Linux there doesn't appear to be any character deformation and I didn't see any box clipping glitches.
On Windows XP it looks better, but the titles do look thinner and fainter. Might be because of the loss of Subpixel AA?
On OS X it's mostly just broken looking (see screenshot). I haven't tested, but it might be due to some interaction between the text-shadow, opacity and background-clip.
This is looking better. I do have some concerns about the loss of subpixel AA. It doesn't look very noticeable in my situation but it's hard to say how this will work on a variety of displays.
Attachment #8753991 -
Attachment is obsolete: true
Flags: needinfo?(shorlander)
Comment 45•9 years ago
|
||
While fixing bug 1269971, jfkthame said[1] I should add -moz-osx-font-smoothing to pass bg-clip:text reftest on OSX:
-moz-osx-font-smoothing:grayscale
this prop might be able to improve text rendering quality on OSX
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1269971#c55
Assignee | ||
Comment 46•9 years ago
|
||
(In reply to Stephen Horlander [:shorlander] from comment #44)
> On Windows XP it looks better, but the titles do look thinner and fainter.
> Might be because of the loss of Subpixel AA?
Yep.
> On OS X it's mostly just broken looking (see screenshot). I haven't tested,
> but it might be due to some interaction between the text-shadow, opacity and
> background-clip.
Since the text is now essentially a background-image, I think what you see is just the text-shadow rendered above the text rather than behind it. Would you be okay with dropping the text-shadow? We'll have the same problem with lightweight themes.
> This is looking better. I do have some concerns about the loss of subpixel
> AA. It doesn't look very noticeable in my situation but it's hard to say how
> this will work on a variety of displays.
Note that we've already been getting grayscale AA in the tab strip on Windows 7 and later for technical reasons. :/ So, no loss there.
Flags: needinfo?(shorlander)
Comment 47•9 years ago
|
||
Note that you could have generated relevant screenshots on a try push with the following try syntax:
> try: -b o -p linux,macosx64,win32,win64 -u mochitest-browser-screenshots[Ubuntu,10.10,Windows XP,Windows 7,Windows 8] -t none --setenv MOZSCREENSHOTS_SETS=Tabs,WindowSize,LightweightThemes
See https://developer.mozilla.org/en-US/docs/Mozilla/QA/Browser_screenshots for more info.
Comment 49•8 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #46)
> (In reply to Stephen Horlander [:shorlander] from comment #44)
> > On Windows XP it looks better, but the titles do look thinner and fainter.
> > Might be because of the loss of Subpixel AA?
>
> Yep.
>
> > On OS X it's mostly just broken looking (see screenshot). I haven't tested,
> > but it might be due to some interaction between the text-shadow, opacity and
> > background-clip.
>
> Since the text is now essentially a background-image, I think what you see
> is just the text-shadow rendered above the text rather than behind it. Would
> you be okay with dropping the text-shadow? We'll have the same problem with
> lightweight themes.
>
> > This is looking better. I do have some concerns about the loss of subpixel
> > AA. It doesn't look very noticeable in my situation but it's hard to say how
> > this will work on a variety of displays.
>
> Note that we've already been getting grayscale AA in the tab strip on
> Windows 7 and later for technical reasons. :/ So, no loss there.
From shorlander on IRC:
8:27 PM <shorlander> can't decide. I think the only real option is to do it on Windows but not on Linux and OS X
Flags: needinfo?(shorlander)
Comment 50•8 years ago
|
||
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #49)
> (In reply to Dão Gottwald [:dao] from comment #46)
> > (In reply to Stephen Horlander [:shorlander] from comment #44)
> > > On Windows XP it looks better, but the titles do look thinner and fainter.
> > > Might be because of the loss of Subpixel AA?
> >
> > Yep.
> >
> > > On OS X it's mostly just broken looking (see screenshot). I haven't tested,
> > > but it might be due to some interaction between the text-shadow, opacity and
> > > background-clip.
> >
> > Since the text is now essentially a background-image, I think what you see
> > is just the text-shadow rendered above the text rather than behind it. Would
> > you be okay with dropping the text-shadow? We'll have the same problem with
> > lightweight themes.
> >
> > > This is looking better. I do have some concerns about the loss of subpixel
> > > AA. It doesn't look very noticeable in my situation but it's hard to say how
> > > this will work on a variety of displays.
> >
> > Note that we've already been getting grayscale AA in the tab strip on
> > Windows 7 and later for technical reasons. :/ So, no loss there.
>
> From shorlander on IRC:
> 8:27 PM <shorlander> can't decide. I think the only real option is to do it
> on Windows but not on Linux and OS X
I guess I should clarify that further and say we shouldn't do it on Windows XP either. I don't think the text rendering regression is worth the trade-off of a few extra visible characters.
Would be great if there was a to do this that didn't affect text rendering or subpixel AA.
Comment 51•8 years ago
|
||
Could we use mask instead of background-clip: text? We support mask with gradient recently, which can be used to implement the fading out effect, and I expect that to be faster than background-clip: text with gradient background.
Flags: needinfo?(dao+bmo)
Comment 52•8 years ago
|
||
mask-image is currently only enabled for non-release build, but I guess we can make it always available for chrome code if that helps.
Comment 53•8 years ago
|
||
Yes, that would be the better solution. It still would not give us subpixel AA, but at least we can keep the text shadow on Mac. And on the platform side we could add support for subpixel AA inside masked elements later.
Updated•8 years ago
|
Assignee | ||
Comment 54•8 years ago
|
||
This patch uses mask-image. Seems to be working well at a first glance.
Attachment #8759143 -
Attachment is obsolete: true
Flags: needinfo?(dao+bmo)
Assignee | ||
Comment 55•8 years ago
|
||
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #52)
> mask-image is currently only enabled for non-release build, but I guess we
> can make it always available for chrome code if that helps.
I still need to run this through talos and see if there's anything else blocking this. If it's otherwise good to go, mask-image always being available for chrome code would certainly be helpful.
No longer depends on: 1264905
Assignee | ||
Updated•8 years ago
|
Attachment #8755930 -
Attachment is obsolete: true
Assignee | ||
Comment 56•8 years ago
|
||
Talos impact looks mostly better than with the previous patch using background-clip (comment 42):
tart opt
osx-10-10 4.00%
windows7-32 3.62%
tart opt e10s
windows7-32 2.91%
tps opt
windows7-32 2.05%
https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=ad41b5cc2cc0&newProject=try&newRevision=5d1a9b06ce28&framework=1
Assignee | ||
Comment 57•8 years ago
|
||
Assignee | ||
Updated•8 years ago
|
Attachment #8793001 -
Flags: ui-review?(shorlander)
Comment 58•8 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #56)
> Talos impact looks mostly better than with the previous patch using
> background-clip (comment 42):
>
> tart opt
> osx-10-10 4.00%
> windows7-32 3.62%
>
> tart opt e10s
> windows7-32 2.91%
>
> tps opt
> windows7-32 2.05%
Looks like we still regress performance on Windows 7 to some extent, and surprisingly it also regresses OS X on tart by a pretty big percentage in addition.
Probably the performance of mask-image isn't that good on OS X, but pretty good on Linux?
Comment 59•8 years ago
|
||
Comment on attachment 8793001 [details] [diff] [review]
patch v3
Review of attachment 8793001 [details] [diff] [review]:
-----------------------------------------------------------------
Tested on Windows 10, macOS 10.12 and Ubuntu. Looks good to me. The loss of sub-pixel AA on some platforms is sad, but otherwise the weight and rendering looks good. Probably worth the trade-off for the 2-3 extra visible characters.
Attachment #8793001 -
Flags: ui-review?(shorlander) → ui-review+
Assignee | ||
Comment 60•8 years ago
|
||
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #58)
> (In reply to Dão Gottwald [:dao] from comment #56)
> > Talos impact looks mostly better than with the previous patch using
> > background-clip (comment 42):
> >
> > tart opt
> > osx-10-10 4.00%
> > windows7-32 3.62%
> >
> > tart opt e10s
> > windows7-32 2.91%
> >
> > tps opt
> > windows7-32 2.05%
>
> Looks like we still regress performance on Windows 7 to some extent, and
> surprisingly it also regresses OS X on tart by a pretty big percentage in
> addition.
>
> Probably the performance of mask-image isn't that good on OS X, but pretty
> good on Linux?
It seems that way. I don't know why we seem to handle this better on Linux. Maybe this has something to do with Skia? Not sure where we're currently use that.
But note that most regressions are for non-e10s. I suspect we'll care less and less about that as we move more users to e10s.
Comment 61•8 years ago
|
||
Be notice that we only enable mask-image on nightly/aurora channels before bug 1251161 fixed.
Comment 62•8 years ago
|
||
(In reply to C.J. Ku[:cjku](UTC+8) from comment #61)
> Be notice that we only enable mask-image on nightly/aurora channels before
> bug 1251161 fixed.
Yes, but enabling it for chrome code in all channels is easy. Adding a flag to nsCSSPropList.h would be enough.
Assignee | ||
Comment 63•8 years ago
|
||
Attachment #8793001 -
Attachment is obsolete: true
Assignee | ||
Comment 64•8 years ago
|
||
Attachment #8813614 -
Attachment is obsolete: true
Assignee | ||
Comment 65•8 years ago
|
||
Comment on attachment 8813627 [details] [diff] [review]
patch v3, browser_overflowScroll.js fixed
I'm seeing a 2.34% tart regression on Windows 7 / e10s. This seems like an acceptable price to pay.
https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=ac149b9c9f5be088eea475447e99f827c7ba63c6&newProject=try&newRevision=4eec2335fcbf6ab9645b6760352aac9d95ea02ab&framework=1
Attachment #8813627 -
Flags: review?(jaws)
Comment 66•8 years ago
|
||
Comment on attachment 8813627 [details] [diff] [review]
patch v3, browser_overflowScroll.js fixed
Review of attachment 8813627 [details] [diff] [review]:
-----------------------------------------------------------------
::: browser/base/content/tabbrowser.xml
@@ -1471,5 @@
> .getService(Components.interfaces.nsITextToSubURI);
> title = textToSubURI.unEscapeNonAsciiURI(characterSet, title);
> } catch (ex) { /* Do nothing. */ }
> -
> - crop = "center";
I would prefer that we still crop in the center for pages without titles and that we don't fade the text in this case. Loading the local file at 'file:///c:/users/msunu/downloads/1f923.svg' crops to 'file:///c:.../1f923.svg' for me, which is quite a bit more useful than if I were to just see 'file:///c:/users/msunu/' on the tab.
Attachment #8813627 -
Flags: review?(jaws) → review+
Assignee | ||
Comment 67•8 years ago
|
||
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #66)
> Comment on attachment 8813627 [details] [diff] [review]
> patch v3, browser_overflowScroll.js fixed
>
> Review of attachment 8813627 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: browser/base/content/tabbrowser.xml
> @@ -1471,5 @@
> > .getService(Components.interfaces.nsITextToSubURI);
> > title = textToSubURI.unEscapeNonAsciiURI(characterSet, title);
> > } catch (ex) { /* Do nothing. */ }
> > -
> > - crop = "center";
>
> I would prefer that we still crop in the center for pages without titles and
> that we don't fade the text in this case. Loading the local file at
> 'file:///c:/users/msunu/downloads/1f923.svg' crops to
> 'file:///c:.../1f923.svg' for me, which is quite a bit more useful than if I
> were to just see 'file:///c:/users/msunu/' on the tab.
As it stands the crop attribute won't work given the layout changes concerning the label element. In theory it should be possible to dynamically switch layout for certain tabs, but that's no simple change. I can file a followup.
Comment 68•8 years ago
|
||
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b323faf96458
Fade out tab label on overflow instead of ellipsis. r=jaws ui-r=shorlander
I had to back this out for failures like https://treeherder.mozilla.org/logviewer.html#?job_id=39757655&repo=mozilla-inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/fa486f98ec41
Flags: needinfo?(dao+bmo)
This also appears to have caused some assertions to fire in various other tests: https://treeherder.mozilla.org/logviewer.html#?job_id=39757821&repo=mozilla-inbound
Comment 71•8 years ago
|
||
(In reply to Wes Kocher (:KWierso) from comment #70)
> This also appears to have caused some assertions to fire in various other tests:
> https://treeherder.mozilla.org/logviewer.html#?job_id=39757821&repo=mozilla-inbound
Flags: needinfo?(cku)
Comment 72•8 years ago
|
||
I'm curious whether [1] is cause by [2]. Please let me if [1] exist after [2] been fixed.
[1]
https://treeherder.mozilla.org/logviewer.html#?job_id=39757821&repo=mozilla-inbound
ASSERTION: How did we getting here, then?: 'aFrame->GetParent()->StyleContext()->GetPseudo() == nsCSSAnonBoxes::mozAnonymousBlock', file /home/worker/workspace/build/src/layout/svg/nsSVGIntegrationUtils.cpp, line 115
[2]
https://treeherder.mozilla.org/logviewer.html#?job_id=39757655&repo=mozilla-inbound
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/tests/mochitest/tree/test_tabbrowser.xul | { pagetab: [ pushbutton: [ ] ] } and ['tab node', address: [object XULElement], role: pagetab, name: 'About:', address: [xpconnect wrapped nsIAccessible]] have different children at index 0 : { pushbutton: [ ] }, ['undefined node', address: [object Text], role: text leaf, name: 'About:', address: [xpconnect wrapped nsIAccessible]]
Flags: needinfo?(cku)
Assignee | ||
Comment 73•8 years ago
|
||
(In reply to C.J. Ku[:cjku](UTC+8) from comment #72)
> I'm curious whether [1] is cause by [2]. Please let me if [1] exist after
> [2] been fixed.
That's very unlikely. accessible/tests/mochitest/tree/test_tabbrowser.xul probably just needs a test-only fix like it did numerous times in the past:
https://hg.mozilla.org/mozilla-central/filelog/34fce7c12173bdd6dda54c2ebf6d344252f1ac48/accessible/tests/mochitest/tree/test_tabbrowser.xul
(I hate this test.)
Flags: needinfo?(dao+bmo) → needinfo?(cku)
Assignee | ||
Comment 74•8 years ago
|
||
Attachment #8813627 -
Attachment is obsolete: true
Assignee | ||
Comment 75•8 years ago
|
||
Comment 76•8 years ago
|
||
I can reproduce this bug on my notebook. Will look into it.
Comment 77•8 years ago
|
||
That assertion is out-of-date. I will fix it in bug 1320364
Flags: needinfo?(cku)
Comment 78•8 years ago
|
||
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6fddc51e9d15
Fade out tab label on overflow instead of ellipsis. r=jaws ui-r=shorlander
Comment 79•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Comment 80•8 years ago
|
||
Release Note Request (optional, but appreciated)
[Why is this notable]: Tabs will look differently than they used to
[Affects Firefox for Android]:no
[Suggested wording]: Shortened titles on tabs are faded out instead of using ellipsis for improved readability
[Links (documentation, blog post, etc)]:
relnote-firefox:
--- → ?
Added to Fx53 Aurora release notes.
Updated•8 years ago
|
Flags: qe-verify+
Comment 82•8 years ago
|
||
Reproduced the bug on 53.0a1 (2016-12-08). I can confirm that the issue is verified fixed on 53.0b1 build1 (20170307064827), using Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.11.6.
Comment 83•8 years ago
|
||
This has caused issues with Rtl text titles. They fade from left side which is not correct.
Should I file another bug?
Comment 84•8 years ago
|
||
(In reply to Salar Khalilzadeh from comment #83)
> This has caused issues with Rtl text titles. They fade from left side which
> is not correct.
> Should I file another bug?
Yes, file a bug please. Thanks.
Comment 85•8 years ago
|
||
> Yes, file a bug please. Thanks.
The bug is here https://bugzilla.mozilla.org/show_bug.cgi?id=1357656
Comment 86•8 years ago
|
||
This is a great little cut but I have a suggestion/request to improve it: at the moment, at least on my machine (Windows 10 hidpi at 200% scaling) it appears that only the last character fades out and I feel that only fading the last character kinda defeats the purpose of this, that was a LOT of work and effort to only fade out the last character! It's so subtle that it's barely noticeable even on a 200% scaling hidpi display, it's not really fulfilling its purpose. Chrome by comparison fades approximately the last 2.5 characters. Please extend the fade! It's far too short and subtle at the moment
I've attached a screenshot comparing Chrome to Firefox fade out
Comment 87•8 years ago
|
||
(In reply to Will from comment #86)
> Created attachment 8865076 [details]
> wikifadecomparison.png
>
> This is a great little cut but I have a suggestion/request to improve it: at
> the moment, at least on my machine (Windows 10 hidpi at 200% scaling) it
> appears that only the last character fades out and I feel that only fading
> the last character kinda defeats the purpose of this, that was a LOT of work
> and effort to only fade out the last character! It's so subtle that it's
> barely noticeable even on a 200% scaling hidpi display, it's not really
> fulfilling its purpose. Chrome by comparison fades approximately the last
> 2.5 characters. Please extend the fade! It's far too short and subtle at the
> moment
>
> I've attached a screenshot comparing Chrome to Firefox fade out
Stephen, do you think we need to change anything here that isn't already covered under photon bugs?
Flags: needinfo?(shorlander)
Assignee | ||
Comment 88•8 years ago
|
||
(In reply to :Gijs from comment #87)
> (In reply to Will from comment #86)
> > Created attachment 8865076 [details]
> > wikifadecomparison.png
> >
> > This is a great little cut but I have a suggestion/request to improve it: at
> > the moment, at least on my machine (Windows 10 hidpi at 200% scaling) it
> > appears that only the last character fades out and I feel that only fading
> > the last character kinda defeats the purpose of this, that was a LOT of work
> > and effort to only fade out the last character! It's so subtle that it's
> > barely noticeable even on a 200% scaling hidpi display, it's not really
> > fulfilling its purpose. Chrome by comparison fades approximately the last
> > 2.5 characters. Please extend the fade! It's far too short and subtle at the
> > moment
> >
> > I've attached a screenshot comparing Chrome to Firefox fade out
>
> Stephen, do you think we need to change anything here that isn't already
> covered under photon bugs?
Stephen filed bug 1363033 on this.
Depends on: 1363033
Updated•8 years ago
|
Flags: needinfo?(shorlander)
Updated•7 years ago
|
Updated•7 years ago
|
Comment 90•7 years ago
|
||
I wish someone would reconsider removing the dropshadow/mask:
.tab-label-container[textoverflow]:not([pinned]) {
mask-image: unset !important;
}
This is not helping it's just making text more unreadable in minimal tabwidth.
Updated•3 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•