The default bug view has changed. See this FAQ.

Fade out tab label on overflow instead of ellipsis

VERIFIED FIXED in Firefox 53

Status

()

Firefox
Tabbed Browser
VERIFIED FIXED
6 years ago
12 days ago

People

(Reporter: Tyler, Assigned: dao)

Tracking

Trunk
Firefox 53
Points:
---
Dependency tree / graph
Bug Flags:
firefox-backlog +

Firefox Tracking Flags

(firefox53 verified, relnote-firefox 53+)

Details

(Whiteboard: p=0 [qx:spec])

Attachments

(1 attachment, 11 obsolete attachments)

(Reporter)

Description

6 years ago
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.
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.
Component: Theme → Tabbed Browser
QA Contact: theme → tabbed.browser

Updated

5 years ago
Component: Tabbed Browser → Theme
See Also: → bug 653670
(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

5 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
(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.
What the UX team is currently thinking about this change ?
(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

5 years ago
Severity: enhancement → normal
Summary: On Tab Overflow fade out instead of ellipses → Fade out tab label on overflow instead of ellipsis
Depends on: 759568
Blocks: 732583
Whiteboard: [Australis:M3]
Perhaps something like http://www.webdesignerwall.com/demo/css-gradient-text/ would work?
(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

4 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

4 years ago
I will email folks and look at this some more the coming week.
Assignee: nobody → gijskruitbosch+bugs

Comment 14

4 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.
(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

4 years ago
See Also: → bug 624595
Whiteboard: [Australis:M3] → [Australis:M6]

Comment 16

4 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..
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.
(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

4 years ago
PSA: Jonathan Watt posted about this issue in m.d.platform: https://groups.google.com/forum/#!topic/mozilla.dev.platform/-23mG3x2vdk
Whiteboard: [Australis:M-]
Depends on: 877294

Updated

4 years ago
Assignee: gijskruitbosch+bugs → nobody
Whiteboard: [Australis:M-] → [Australis:M-][Australis:P?]
Removing from the Australis project. Still want, but not part of Australis.
No longer blocks: 732583
Whiteboard: [Australis:M-][Australis:P?]

Updated

3 years ago
Blocks: 950073
Whiteboard: [triage]

Updated

3 years ago
Whiteboard: [triage]

Updated

3 years ago
No longer blocks: 950073

Updated

3 years ago
Blocks: 950073
Whiteboard: [feature] p=0

Updated

3 years ago
No longer blocks: 950073
Flags: firefox-backlog+
Whiteboard: [feature] p=0 → p=0
Whiteboard: p=0 → p=0 [qx]
Whiteboard: p=0 [qx] → p=0 [qx:spec]
(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)

(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)
Thanks!
(Assignee)

Updated

a year ago
Depends on: 1224422
No longer depends on: 877294
(Assignee)

Comment 24

11 months ago
Created attachment 8741814 [details] [diff] [review]
WIP patch

This works with layout.css.background-clip-text.enabled = true
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
(Assignee)

Updated

11 months ago
Depends on: 1259345
\o/ So glad we're finally getting around to this.
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

11 months 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.
Depends on: 1264905
No longer depends on: 1259345, 759568
(Assignee)

Comment 28

11 months ago
Created attachment 8741946 [details] [diff] [review]
WIP patch 2
Attachment #8741814 - Attachment is obsolete: true
(Assignee)

Updated

11 months ago
No longer depends on: 1224422

Comment 29

11 months 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

11 months ago
Created attachment 8742079 [details] [diff] [review]
WIP patch 3

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

11 months ago
Created attachment 8742097 [details] [diff] [review]
patch

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

11 months ago
Blocks: 624595
See Also: bug 624595
(Assignee)

Updated

11 months ago
Component: Theme → Tabbed Browser
(Assignee)

Comment 32

11 months ago
Created attachment 8742103 [details] [diff] [review]
patch v2

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

11 months 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 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

11 months 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)
(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)
Will try and test on Linux and Windows 10, but I am having some build issues.
(Assignee)

Comment 38

10 months 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)
(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.
Created attachment 8753991 [details]
Tab Title Glitches (Windows 10)

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

10 months 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

10 months 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

10 months 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)
Created attachment 8755930 [details]
Tab Fade Screenshot - Patch 02

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

10 months 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

10 months 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)
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.
(Assignee)

Comment 48

10 months ago
Created attachment 8759143 [details] [diff] [review]
patch v2

rebased
Attachment #8742103 - Attachment is obsolete: true
(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)
(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.
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)
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.
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.
See Also: → bug 898984
(Assignee)

Comment 54

6 months ago
Created attachment 8793001 [details] [diff] [review]
patch v3

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

6 months 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

6 months ago
Attachment #8755930 - Attachment is obsolete: true
(Assignee)

Comment 56

6 months 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

6 months ago
Try builds:

https://archive.mozilla.org/pub/firefox/try-builds/dgottwald@mozilla.com-5d1a9b06ce285b237099751c5a194f616f093813/
(Assignee)

Updated

6 months ago
Attachment #8793001 - Flags: ui-review?(shorlander)
(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 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

6 months 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.
Be notice that we only enable mask-image on nightly/aurora channels before bug 1251161 fixed.
(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)

Updated

6 months ago
Depends on: 1304698
Depends on: 1305259
(Assignee)

Updated

6 months ago
Depends on: 1251161
(Assignee)

Comment 63

4 months ago
Created attachment 8813614 [details] [diff] [review]
patch v3, rebased
Attachment #8793001 - Attachment is obsolete: true
(Assignee)

Comment 64

4 months ago
Created attachment 8813627 [details] [diff] [review]
patch v3, browser_overflowScroll.js fixed
Attachment #8813614 - Attachment is obsolete: true
(Assignee)

Comment 65

4 months 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 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

4 months 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

4 months 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
(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)
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

4 months 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

4 months ago
Created attachment 8814057 [details] [diff] [review]
patch v4, test_tabbrowser.xul fixed
Attachment #8813627 - Attachment is obsolete: true
(Assignee)

Comment 75

4 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a47b7c8675fa6d76f60861e9086f2a0ab55e5ec2
I can reproduce this bug on my notebook. Will look into it.

Updated

4 months ago
Depends on: 1320364
That assertion is out-of-date. I will fix it in bug 1320364
Flags: needinfo?(cku)

Comment 78

4 months 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

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6fddc51e9d15
Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
(Assignee)

Updated

3 months ago
Depends on: 1322889
Depends on: 1322897

Updated

3 months ago
Depends on: 1323057
Depends on: 1323125
(Assignee)

Updated

3 months ago
Blocks: 1323134
(Assignee)

Updated

3 months ago
No longer depends on: 1304698
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: --- → ?

Comment 81

2 months ago
Added to Fx53 Aurora release notes.
relnote-firefox: ? → 53+

Updated

2 months ago
Depends on: 1335774

Updated

2 months ago
Depends on: 1337293
No longer depends on: 1337293
Flags: qe-verify+
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.
Status: RESOLVED → VERIFIED
status-firefox53: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.