Sylvestre Ledru [:Sylvestre] Reporter
	Description • 10 years ago

As you can see in the two screenshots, the rendering of the highlighting targets can be bad.
Please note that I am using a Awesome (A Window Manager)

	:Gijs (he/him)
	Comment 2 • 10 years ago

Considering this is likely a layout/gfx/window manager issue, and the relative number of users who are likely to see this, I don't think this should be a P3.

	Matthew N. [:MattN] Assignee
	Comment 3 • 10 years ago

ato on IRC says this also happens with fvwm on Ubuntu.

	:Gijs (he/him)
	Comment 4 • 10 years ago

Out of curiosity, does this not affect other semi-transparent panel things, like the box shadows we have on panels?

	Sylvestre Ledru [:Sylvestre] Reporter
	Comment 5 • 10 years ago

No problem here (bookmark menu)

	Andreas Tolfsen ❲:ato❳
	Comment 6 • 10 years ago

Attaching screenshot on fvwm.  I can't say I see the problem with the shadow in the bookmark panel, but then I don't know exactly what I'm looking for.

	Andreas Tolfsen ❲:ato❳
	Comment 7 • 10 years ago

It turns out that fvwm, and presumably also awesome, are not running X compositors by default.  Starting up a compositor program like compton or xcompmgr solves the problem.  I'm attaching a screenshot to prove this.

The question then is what a sensible fallback should be for window managers who deliberately choose not to use one.  A circular ring without shadow and opacity wouldn't look too bad.

That said, I'm inclined to think that this is such a marginal case that doing a fallback might not be worth the effort.

	Sylvestre Ledru [:Sylvestre] Reporter
	Comment 8 • 10 years ago

I can confirm that using compton fixed the problem with awesome.

	Matthew N. [:MattN] Assignee
	Comment 10 • 10 years ago

Karl, is this something you can take a look at or do you know who would know whether there is something better we can do for users without X compositors?

	Karl Tomlinson (:karlt)
	Comment 11 • 10 years ago

When there is no window compositor running, alpha values are rounded to 0 or 1.  (127/255 is rounded down and 128/255 is rounded up.)

The simplest solution might be to adjust the highlighting panel so that the 0.5 contour in the alpha values is at a reasonable place to cut off to/from transparency.

Do you know what avoids this problem on Windows systems when there is no compositor?

	Matthew N. [:MattN] Assignee
	Comment 12 • 10 years ago

(In reply to Karl Tomlinson (:karlt) from comment #11)
> When there is no window compositor running, alpha values are rounded to 0 or
> 1.  (127/255 is rounded down and 128/255 is rounded up.)
> 
> The simplest solution might be to adjust the highlighting panel so that the
> 0.5 contour in the alpha values is at a reasonable place to cut off to/from
> transparency.

I see. We can give that a try but I suspect it still seems like there is potential for some kind of fallback in GFX code.

> Do you know what avoids this problem on Windows systems when there is no
> compositor?

I know very little about our layout => graphics pipeline or GFX backends to be honest. Who would be able to look into a "proper" solution if one like the Windows one exists?

	Karl Tomlinson (:karlt)
	Comment 13 • 10 years ago

(In reply to Matthew N. [:MattN] from comment #12)
> I suspect it still seems like there is
> potential for some kind of fallback in GFX code.

If there is no window compositor running, then gfx code can't do anything to add translucency to windows.

The options are:

1) Using an element in the same window as the highlighted feature, instead of a panel, which is a separate window.

2) Making the panel different depending on whether there is a compositor running or not.  This would require exposing this information to the panel somehow, through a CSS selector or LookAndFeel perhaps.  When there is no compositor running, a simpler panel without translucency would be used.

Each of these is probably too much work for the Australis UI Tour.

> I know very little about our layout => graphics pipeline or GFX backends to
> be honest. Who would be able to look into a "proper" solution if one like
> the Windows one exists?

A quick look at WS_EX_LAYERED documentation suggests that this is always available on systems since Windows 2000, so I guess there is always a compositor available and no fallback is require there.  i.e. the Windows solutions won't apply to the X11 situation.

Even if there were a solution, I don't think Mozilla would be able to prioritize this enough to assign someone.

	Matthew N. [:MattN] Assignee
	Comment 14 • 10 years ago

Okay, thanks for the detailed explanation/investigation Karl.

(In reply to Karl Tomlinson (:karlt) from comment #13)
> 1) Using an element in the same window as the highlighted feature, instead
> of a panel, which is a separate window.

I don't think we could prioritize this on the front-end for Linux-only either as it would mean re-implementing how highlights work (possibly a different implementation for panel highlights) and it would put restrictions on the bounds of highlights as some highlights can go outside the bounds of the panel being highlighted when there is an animation (e.g. currently the wobble animation on the customize icon wobbles to the edge of the menu panel).

Michael, it seems like a sizable population of Linux users will run into this issue. Could you come up with a simplified/adjusted highlight design for Linux-only taking the information from comment 11 into account? Unfortunately we can't detect when we need to use this design so we'll have to use it all times on Linux. The current CSS is here: https://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/UITour.inc.css?rev=542bdf3f5191#17

	Matthew N. [:MattN] Assignee
	Comment 16 • 10 years ago

For anyone who needs to be able to test this or switch off compositing, I found this useful: 

Switch to metacity (disable compositing)
metacity --replace &

Switch to compiz (enable compositing)
compiz --replace &

Source: https://askubuntu.com/questions/36654/how-do-i-turn-on-and-off-compositing-from-the-command-line#answer-37086

	Matthew N. [:MattN] Assignee
	Comment 17 • 10 years ago

It seems like there is no anti-aliasing in this case so I made the border thicker so it didn't look broken/detached in some parts.

	Matthew N. [:MattN] Assignee
	Comment 18 • 10 years ago

Michael, I wasn't able to just shift the opacity values of the gradient down below 0.5 as the aliased edges of the border seemed to be blended with a dark color and made it look bad. It seems like a darker color would be required to make that dark edge blend nicely so I chose one of the colors from the gradient as a starting point. I also had to make the border thicker so that it was a connected circle with aliasing.

	Matthew N. [:MattN] Assignee
	Comment 19 • 10 years ago

Comment on attachment 8401674 [details] [diff] [review]
v.1 Disable animation, thicken border and make it blue, and adjust opacity

[14:55:41]  <mmaslaney>	 Considering the graphic limitations, I feel that it works.

	Michael Maslaney [:mmaslaney] (mmaslaney@mozilla.com)
	Comment 20 • 10 years ago

Comment on attachment 8401630 [details]
Screenshot of changed style without an X compositor

Is this what the majority of Linux user's will be seeing? Anyway to smooth out the pixelation?

	Matthew N. [:MattN] Assignee
	Comment 21 • 10 years ago

(In reply to mmaslaney from comment #20)
> Comment on attachment 8401630 [details]
> Screenshot of changed style without an X compositor
> 
> Is this what the majority of Linux user's will be seeing? Anyway to smooth
> out the pixelation?

I honestly don't know what is the majority. This is a solid line and I don't know of a way to make it smoother since we can't use opacity for that.

	Karl Tomlinson (:karlt)
	Comment 22 • 10 years ago

This looks a very good effort to me.  I expect more users will see the version with the compositor.

Without the compositor, it is not possible to get smooth edges on a circle (because no anti-aliasing is available).

I fear that any effort to remove the dark edges for the without-compositor case will make things less smooth with a compositor.

	Blair McBride [:Unfocused] (UNAVAILABLE)
	Comment 23 • 10 years ago

Comment on attachment 8401674 [details] [diff] [review]
v.1 Disable animation, thicken border and make it blue, and adjust opacity

Review of attachment 8401674 [details] [diff] [review]:
-----------------------------------------------------------------

Oh fun.

(I'm out of the country at conferences this week, and the hotel wifi is rather horrendous here - so I can't remote into a linux VM back home to test this myself. Trusting the attached screenshots here.)

	Matthew N. [:MattN] Assignee
	Comment 24 • 10 years ago

The screenshots are not a lie :P

	Matthew N. [:MattN] Assignee
	Comment 25 • 10 years ago

Comment on attachment 8401674 [details] [diff] [review]
v.1 Disable animation, thicken border and make it blue, and adjust opacity

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 936187
User impact if declined: Ugly highlights for Linux users without a compositor
Testing completed (on m-c, etc.): Local linux VM
Risk to taking this patch (and alternatives if risky): Low risk CSS change of opacity to get nicer fallback and color to deal with dark aliased edges previously around a white border
String or IDL/UUID changes made by this patch: None

Note that I don't think this really needs to wait for m-c landing to get approval since it won't get much Nightly exposure since most Nightly users have already seen the tour and likely won't manually test again.

	Michael Maslaney [:mmaslaney] (mmaslaney@mozilla.com)
	Comment 26 • 10 years ago

Comment on attachment 8401630 [details]
Screenshot of changed style without an X compositor

If we make the highlights, in this particular instance, rounded rectangles or complete squares, would we get the same anti-aliasing effect?

Understandably, we need to move this forward, but I'm a little concerned with the quality highlight as it does impact the overall quality of the tour experience.

	Matthew N. [:MattN] Assignee
	Comment 27 • 10 years ago

(In reply to mmaslaney from comment #26)
> Comment on attachment 8401630 [details]
> Screenshot of changed style without an X compositor
> 
> If we make the highlights, in this particular instance, rounded rectangles
> or complete squares, would we get the same anti-aliasing effect?

Squares wouldn't have a problem, rounded rectangles would only have the aliasing visible on the rounded corners. Note that shape changes would apply to all Linux users, not just ones who have no X compositor.

> Understandably, we need to move this forward, but I'm a little concerned
> with the quality highlight as it does impact the overall quality of the tour
> experience.

An alternative is to hide the highlight altogether for people with the fallback by dropping all opacities below 0.5.

This should really land in Monday's beta so if I don't hear from you by tomorrow I'm going to land this patch since it's better than what we currently have.

	Ryan VanderMeulen [:RyanVM]
	Comment 28 • 10 years ago

https://hg.mozilla.org/integration/fx-team/rev/ba140cc50f04

	Ryan VanderMeulen [:RyanVM]
	Comment 29 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/ba140cc50f04

	Matthew N. [:MattN] Assignee
	Comment 30 • 10 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/5f65df850a44

	Matthew N. [:MattN] Assignee
	Comment 31 • 10 years ago

https://hg.mozilla.org/releases/mozilla-beta/rev/8e6041de3ce7

Michael, please file a new bug blocking this one if you want further changes. Thanks

	Michael Maslaney [:mmaslaney] (mmaslaney@mozilla.com)
	Comment 32 • 10 years ago

Matt, let's change the highlight to a 2px round rectangle for all Linux users. I feel that would be our safest move to ensure the highlight quality.

	Bogdan Maris, Desktop QA
	Comment 33 • 10 years ago

Reproduced the initial issue on Firefox 29 beta 4 using Ubuntu 13.04 32/64bit with Awesome and fvwm and verified that the issue is fixed on Firefox 29 beta 8, latest Aurora and latest Nightly.