Closed Bug 1567094 Opened 2 months ago Closed Last month

WebRender doesn't look at visited-dependent border colors.

Categories

(Core :: Graphics: WebRender, defect)

68 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla70
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- disabled
firefox68 --- wontfix
firefox69 --- verified
firefox70 --- verified

People

(Reporter: andye23, Assigned: emilio)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: correctness, regression)

Attachments

(4 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/75.0.3770.142 Safari/537.36

Steps to reproduce:

html:
<a href="http://foo.de">
<div class="indicator"></div>
</a>
css:
a:visited .indicator {
border-top: 5px solid gray;
}

Actual results:

In earlier versions of FireFox the "indicator" is colored gray, if the link is clicked.

Expected results:

the style of the child element on a pseudo-class should be applied

Hi andye23,

Could you share this code in a test page and the steps to reproduce the reported issue on our end?

Also, please test if you can reproduce this on the latest Firefox Nightly version, you can download it from here https://nightly.mozilla.org/and share the results with us.

Thank you for your report.

Flags: needinfo?(andye23)

Hi,

sorry for the late answer.

i have made a minimum fiddle:
https://jsfiddle.net/zmwn1t7e/3/

Also wrong in FF Developer 69.0b9 (64-bit)

Working in Chorme and Egde as ecspected.

with regards

Flags: needinfo?(andye23)

expectation: the yellow border from the .indicator is switching to gray if link is visited.

It does also not work if it is not a child element.
https://jsfiddle.net/qpvngLxj/

with regards

Flags: needinfo?(tgrabowski)

Hi andye23,

I understand that the expected result is: clicking one element, the yellow border should switch to gray if visited. I was able to reproduce the actual result (border not changing color) in the latest Nightly 70.0a1 (2019-08-13) (64-bit).

I'll mark this ticket as new and assign a component so the corresponding team can take a look at this. Feel free to change it if you think it's not the correct one.

Regards,

Status: UNCONFIRMED → NEW
Component: Untriaged → CSS Parsing and Computation
Ever confirmed: true
Product: Firefox → Core
Flags: needinfo?(tgrabowski)

Hey, thanks a lot for the report, I can repro.

Can you attach your about:support, or copying what you see in the Compositing section? Just to double-check my observations.

This works with WebRender disabled, and doesn't with WebRender enabled. You can see in about:support, in the graphics section, whether it says: Compositing WebRender or not.

Flags: needinfo?(andye23)

Simpler test-case: data:text/html,<a style="display: block; border: 1px solid" href="https://bugzilla.mozilla.org">Foo

The border should be purple.

Component: CSS Parsing and Computation → Graphics: WebRender
Summary: visited pseudo-class does not work on child elements anymore → WebRender doesn't look at visited-dependent border colors.

Hello,

yes, in about:support is Compositing: WebRender.
sorry, the simplert test case i do not understand.

Flags: needinfo?(andye23)

Hello,

i think i understand.
i have made a new fiddle.
https://jsfiddle.net/54wnpexs/

on chrome the border is purple if visited.
on FF the borders are all blue.

Flags: needinfo?(jbonisteel)

Right, this is not about nested stuff or not, it's about border-color and visited. :visited has a bunch of restrictions due to privacy and that's why it is a bit special.

Anyhow I can take this, should be an easy fix.

Please don't spam needinfos unnecessarily.

Flags: needinfo?(jbonisteel)
Assignee: nobody → emilio

ComputedStyle* aComputedStyle doesn't provide any extra value over just aStyle.

Lots of these should be const and what not, oh well.

We do have test coverage for this
(layout/style/test/test_visited_reftests.html), but it seems that that uses
snapshotWindow() / drawWindow() and that may not use the WR code paths? It seems
we may be missing a bit of test coverage there. Is this expected?

Hello,

i have tried to disable the WebRenderer.
But in about:config the "gfx.webrender.all" is set to false.

so how can i disable the Compositing: WebRender?

You need a browser restart. And you may need to set gfx.webrender.force-disabled to true.

mozregression --good 2018-07-15 --bad 2019-01-15 --pref gfx.webrender.all:true browser.startup.homepage:'https://fiddle.jshell.net/zmwn1t7e/3/show/'

11:10.33 INFO: Last good revision: f8aaaf974be99cdf4d1bf4671a9359d142381f3d
11:10.33 INFO: First bad revision: deb35d6b4f8301f15a27a1546dafe7aa8d425ce3
11:10.33 INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=f8aaaf974be99cdf4d1bf4671a9359d142381f3d&tochange=deb35d6b4f8301f15a27a1546dafe7aa8d425ce3

deb35d6b4f8301f15a27a1546dafe7aa8d425ce3 Timothy Nikkel — Bug 1345388. Handle TextDrawTarget fallback. r=mattwoodrow
6f34ff46ab954dab8673679cab663718547f94ed Timothy Nikkel — Bug 1345388. Save using one extra clip. r=aosmond
3878d1245a34e912a375cd54bcae08f9f9495d44 Kevin Chen — Bug 1345388. Draw the alt feedback of an image with WedRender so we don't have to use fallback. r=aosmond
a83b545fe5dd87030c0c2d8f6a1841886ca34965 Kevin Chen — Bug 1345388. Factor out CreateWebRenderCommandsForBorderWithStyleBorder so we can pass an nsStyleBorder of our choice. r=aosmond

Status: NEW → ASSIGNED
Has Regression Range: --- → yes
Has STR: --- → yes
OS: Unspecified → All
Regressed by: 1345388
Hardware: Unspecified → All
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d678d6d2ed75
Fix non-unified build in nsCSSRendering.cpp. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/04c2bfb6dbc6
Rename some arguments to avoid being unnecessarily verbose. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/32c8752fa896
Make GetVisitedDependentColor const-friendly. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/10439459ebd6
Make WebRender look at visited dependent border colors. r=jrmuizel

Given that we're less than a week from Fx69 RC, I think we can let this fix ride Fx70 to release. Feel free to nominate for Beta approval if you feel strongly otherwise.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #14)

We do have test coverage for this
(layout/style/test/test_visited_reftests.html), but it seems that that uses
snapshotWindow() / drawWindow() and that may not use the WR code paths? It seems
we may be missing a bit of test coverage there. Is this expected?

This doesn't seem good. Is there a bug tracking this?

Flags: needinfo?(emilio)
Flags: needinfo?(emilio)

Jessie / Matt (given Jeff is on PTO), do you think this is worth uplifting to beta? I don't think it's too risky, most of it is just moving code around, but...

Flags: needinfo?(matt.woodrow)
Flags: needinfo?(jbonisteel)

Seems low risk to me, and a nice webcompat fix, so I think it's worth uplifting.

Flags: needinfo?(matt.woodrow)

OK, please nominate this for Beta approval when you get a chance.

Flags: needinfo?(jbonisteel) → needinfo?(emilio)

Comment on attachment 9085391 [details]
Bug 1567094 - Make WebRender look at visited dependent border colors.

Beta/Release Uplift Approval Request

  • User impact if declined: Some colors on visited links don't work.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: Go to the link in comment 2 with WebRender enabled, clicking on the links should make the borders turn from yellow to grey.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Mostly refactoring, low-risk change for otherwise well-tested code. bug 1573811 tracks the issue that allowed this bug to exist in the first place.
  • String changes made/needed: none
Flags: needinfo?(emilio)
Attachment #9085391 - Flags: approval-mozilla-beta?
Attachment #9085388 - Flags: approval-mozilla-beta?
Attachment #9085389 - Flags: approval-mozilla-beta?
Attachment #9085390 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Comment on attachment 9085388 [details]
Bug 1567094 - Fix non-unified build in nsCSSRendering.cpp.

Low-risk webcompat fix for users with WebRender enabled. Approved for 69.0b16.

Attachment #9085388 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9085389 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9085390 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9085391 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

Backed out changeset b96a05e1dc18 (Bug 1567094) for build bustage at build/src/layout/painting/nsCSSRendering.cpp

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&selectedJob=262631247&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=0a5ba27e416309d1d68b1b2fc539496731d52ae3

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=262631224&repo=mozilla-beta&lineNumber=24772

Backout link: https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=c6b3e3f56fa5b7ba3ad81f30f2cfa7cab0340303

[task 2019-08-21T09:55:40.660Z] 09:55:40     INFO -  docshell/base/Unified_cpp_docshell_base0.o
[task 2019-08-21T09:55:40.660Z] 09:55:40     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/docshell/base'
[task 2019-08-21T09:55:42.312Z] 09:55:42     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/layout/painting'
[task 2019-08-21T09:55:42.312Z] 09:55:42     INFO -  /builds/worker/workspace/build/src/clang/bin/clang++ -m32 -o Unified_cpp_layout_painting0.o -c  -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -DDEBUG=1 -DOS_POSIX=1 -DOS_LINUX=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/layout/painting -I/builds/worker/workspace/build/src/obj-firefox/layout/painting -I/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -I/builds/worker/workspace/build/src/ipc/chromium/src -I/builds/worker/workspace/build/src/ipc/glue -I/builds/worker/workspace/build/src/docshell/base -I/builds/worker/workspace/build/src/dom/base -I/builds/worker/workspace/build/src/layout/base -I/builds/worker/workspace/build/src/layout/generic -I/builds/worker/workspace/build/src/layout/style -I/builds/worker/workspace/build/src/layout/svg -I/builds/worker/workspace/build/src/layout/tables -I/builds/worker/workspace/build/src/layout/xul -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -Qunused-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++1z-compat -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -Wno-return-type-c-linkage -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fcrash-diagnostics-dir=/builds/worker/artifacts -march=pentium-m -msse -msse2 -mfpmath=sse -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/build/src/obj-firefox/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -Os -fno-omit-frame-pointer -funwind-tables -Werror -I/builds/worker/workspace/build/src/obj-firefox/dist/include/cairo -Wno-error=shadow  -MD -MP -MF .deps/Unified_cpp_layout_painting0.o.pp   /builds/worker/workspace/build/src/obj-firefox/layout/painting/Unified_cpp_layout_painting0.cpp
[task 2019-08-21T09:55:42.312Z] 09:55:42     INFO -  In file included from /builds/worker/workspace/build/src/obj-firefox/layout/painting/Unified_cpp_layout_painting0.cpp:101:
[task 2019-08-21T09:55:42.312Z] 09:55:42     INFO -  /builds/worker/workspace/build/src/layout/painting/nsCSSRendering.cpp:16:10: fatal error: 'mozilla/StaticPrefs_layout.h' file not found
[task 2019-08-21T09:55:42.312Z] 09:55:42     INFO -  #include "mozilla/StaticPrefs_layout.h"
[task 2019-08-21T09:55:42.312Z] 09:55:42     INFO -           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[task 2019-08-21T09:55:42.312Z] 09:55:42     INFO -  1 error generated.
[task 2019-08-21T09:55:42.312Z] 09:55:42     INFO -  /builds/worker/workspace/build/src/config/rules.mk:801: recipe for target 'Unified_cpp_layout_painting0.o' failed
[task 2019-08-21T09:55:42.312Z] 09:55:42    ERROR -  make[4]: *** [Unified_cpp_layout_painting0.o] Error 1
[task 2019-08-21T09:55:42.312Z] 09:55:42     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/layout/painting'
[task 2019-08-21T09:55:42.312Z] 09:55:42     INFO -  /builds/worker/workspace/build/src/config/recurse.mk:74: recipe for target 'layout/painting/target' failed
[task 2019-08-21T09:55:42.312Z] 09:55:42    ERROR -  make[3]: *** [layout/painting/target] Error 2
[task 2019-08-21T09:55:42.312Z] 09:55:42     INFO -  make[3]: *** Waiting for unfinished jobs....
[task 2019-08-21T09:55:42.312Z] 09:55:42     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/docshell/base'
[task 2019-08-21T09:55:42.312Z] 09:55:42     INFO -  docshell/base/Unified_cpp_docshell_base1.o
[task 2019-08-21T09:55:42.312Z] 09:55:42     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/docshell/base'
[task 2019-08-21T09:55:45.026Z] 09:55:45     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/widget'
[task 2019-08-21T09:55:45.026Z] 09:55:45     INFO -  /builds/worker/workspace/build/src/clang/bin/clang++ -m32 -o Unified_cpp_widget2.o -c  -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -DDEBUG=1 -DMOZ_CROSS_PROCESS_IME -DOS_POSIX=1 -DOS_LINUX=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/widget -I/builds/worker/workspace/build/src/obj-firefox/widget -I/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -I/builds/worker/workspace/build/src/ipc/chromium/src -I/builds/worker/workspace/build/src/ipc/glue -I/builds/worker/workspace/build/src/dom/base -I/builds/worker/workspace/build/src/dom/ipc -I/builds/worker/workspace/build/src/layout/base -I/builds/worker/workspace/build/src/layout/forms -I/builds/worker/workspace/build/src/layout/generic -I/builds/worker/workspace/build/src/layout/painting -I/builds/worker/workspace/build/src/layout/xul -I/builds/worker/workspace/build/src/layout/xul/tree -I/builds/worker/workspace/build/src/view -I/builds/worker/workspace/build/src/widget -I/builds/worker/workspace/build/src/widget/headless -I/builds/worker/workspace/build/src/widget/gtk -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -Qunused-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++1z-compat -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -Wno-return-type-c-linkage -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fcrash-diagnostics-dir=/builds/worker/artifacts -march=pentium-m -msse -msse2 -mfpmath=sse -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/build/src/obj-firefox/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -Os -fno-omit-frame-pointer -funwind-tables -Werror -I/builds/worker/workspace/build/src/widget/gtk/compat-gtk3 -pthread -I/usr/include/gtk-3.0 -I/usr/include/atk-1.0 -I/usr/include/at-spi2-atk/2.0 -I/usr/include/pango-1.0 -I/usr/include/gio-unix-2.0/ -I/usr/include/cairo -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/glib-2.0 -I/usr/lib/i386-linux-gnu/glib-2.0/include -I/usr/include/harfbuzz -I/usr/include/freetype2 -I/usr/include/pixman-1 -I/usr/include/libpng12 -I/usr/include/gtk-3.0/unix-print -pthread -I/usr/include/gtk-3.0 -I/usr/include/atk-1.0 -I/usr/include/at-spi2-atk/2.0 -I/usr/include/pango-1.0 -I/usr/include/gio-unix-2.0/ -I/usr/include/cairo -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/glib-2.0 -I/usr/lib/i386-linux-gnu/glib-2.0/include -I/usr/include/harfbuzz -I/usr/include/freetype2 -I/usr/include/pixman-1 -I/usr/include/libpng12 -I/usr/include/libdrm  -MD -MP -MF .deps/Unified_cpp_widget2.o.pp   /builds/worker/workspace/build/src/obj-firefox/widget/Unified_cpp_widget2.cpp
[task 2019-08-21T09:55:45.026Z] 09:55:45     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/widget'
[task 2019-08-21T09:55:45.026Z] 09:55:45     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/layout/style'
[task 2019-08-21T09:55:45.026Z] 09:55:45     INFO -  layout/style/Unified_cpp_layout_style3.o
Flags: needinfo?(emilio)

We agreed on backing out that changeset (https://hg.mozilla.org/releases/mozilla-beta/rev/b96a05e1dc18) since the file being included doesn't exist yet in beta. It shouldn't affect the fix anyhow.

Flags: needinfo?(emilio)

Confirmed issue with 69.0b15.
Fix verified with 70.0a1 (2019-08-20).

Status: RESOLVED → VERIFIED

Verified with 69.0b16 as well

You need to log in before you can comment on or make changes to this bug.