Closed Bug 1323404 Opened 7 years ago Closed 7 years ago
[Stylo] Stylo renders black text as grey
Check this reftest result: https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=https://queue.taskcluster.net/v1/task/DWDN_wJjTlOG63eRyp3h6Q/runs/0/artifacts/public/logs/live_backing.log&only_show_unexpected=1 Select the "file:///home/worker/workspace/build/tests/reftest/tests/layout/reftests/reftest-sanity/default.html" test case. The "Image 1 (TEST) is from Stylo, the text is grey; The "Image 2(REFERENCE)" is from Gecko's stock style system, the text is black. There are tons of reftest failures because of this color issue. (See https://treeherder.mozilla.org/#/jobs?repo=try&revision=82a813d1bd1df2a9c4e3a0733c6de62d08f27a44&selectedJob=32641575 for a full result)
OS: Unspecified → Linux
Hardware: Unspecified → x86_64
Interesting! Curious to know what's causing this. Is it related to our hackiness here? http://searchfox.org/mozilla-central/rev/36bfd0a8da5490274ca49c100957244253845e52/layout/style/StyleStructContext.h#81 If so, it might get fixed by bz's work in bug 1298588. But I'd be sorta surprised if > LookAndFeel::GetColor(LookAndFeel::eColorID_WindowForeground, NS_RGB(0x00, 0x00, 0x00)) Returned gray?
Who knows what that returns, especially on different OSes... What OS _is_ this on? What does Gecko do on that OS if you set the "browser.display.use_system_colors" pref to true?
Oh, I see! I misread the code when implementing StyleStructContext.cpp. I thought that  was the default case, but we actually only hit that when usePrefColors is false, which is not the default. So as a stopgap, we should change the code at  to just return NS_RGB(0x00, 0x00, 0x00) in the SERVO_DEFAULT case. Shing, can you see if that solves the problem?  http://searchfox.org/mozilla-central/rev/36bfd0a8da5490274ca49c100957244253845e52/layout/base/nsPresContext.cpp#466  http://searchfox.org/mozilla-central/rev/36bfd0a8da5490274ca49c100957244253845e52/layout/style/StyleStructContext.h#83
(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #2) > Who knows what that returns, especially on different OSes... What OS _is_ > this on? % uname -a Linux shinglyu-XPS-15-9550 4.4.0-53-generic #74-Ubuntu SMP Fri Dec 2 15:59:10 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux > What does Gecko do on that OS if you set the > "browser.display.use_system_colors" pref to true? I tried, it's still gery
> I tried, it's still gery "still"? I thought you said Gecko's styling was black? To be clear, my question is what happens in Gecko _without_ stylo but with that pref set.
(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #5) > To be clear, my question is what happens in Gecko _without_ stylo but with > that pref set. Oh, Gecko without stylo with that perf on => grey text It also happens on try server (platform = linux64-stylo), so probably not just my computer's issue (In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #3) > So as a stopgap, we should change the code at  to just return > NS_RGB(0x00, 0x00, 0x00) in the SERVO_DEFAULT case. Shing, can you see if > that solves the problem? Yes it fix the problem. Should I submit a patch as workaround or should we dig into the root cause?
Let's land that patch, bz is fixing the root cause.
This will reduce the failures from 4198 to 3044, that's 1154 test cases fixed.
Comment on attachment 8819194 [details] Bug 1323404 - Fixed Stylo renders black text as grey issue https://reviewboard.mozilla.org/r/99060/#review99302
Attachment #8819194 - Flags: review?(bobbyholley) → review+
One other thing I notice is that the URL bar selection background is grey too. This looks related but I'm not quite sure.
> Oh, Gecko without stylo with that perf on => grey text OK, that means that system color on those OSes is in fact gray. ;)
I don't have Lv 3 access (maybe I should get one). Do you mind clicking the land button for me? Thank you. :)
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/f2f24b4c8c2f Fixed Stylo renders black text as grey issue r=bholley
I did it since Bobby is probably already or close to be in bed.
You need to log in before you can comment on or make changes to this bug.