Closed
Bug 1382896
Opened 7 years ago
Closed 6 years ago
Consider simplifying groove / ridge / inset / outset border rendering to match Chrome.
Categories
(Core :: Web Painting, enhancement, P3)
Core
Web Painting
Tracking
()
RESOLVED
FIXED
mozilla64
Tracking | Status | |
---|---|---|
firefox64 | --- | fixed |
People
(Reporter: gw, Assigned: emilio)
References
(Blocks 2 open bugs)
Details
Attachments
(3 files)
The function NS_GetSpecial3DColors attempts to use the background color to tweak the border colors for groove / ridge / inset / outset borders. The code here http://searchfox.org/mozilla-central/source/layout/painting/nsCSSRendering.cpp#714 attempts to find the relevant background color. This adds some complexity to border rendering, and is different from what Chrome does (I don't believe this is specified). See https://github.com/servo/webrender/issues/1280 for some test cases and screenshots of various styles and colors in Gecko, Chrome and Servo. If we remove the Gecko code that tweaks the colors based on the background color, it will mean: * Matching what Chrome does in this case. * Simplifying the WebRender implementation of 3D borders (which is currently very close to Chrome). * Removing some very old code and slightly simplifying the border rendering code in Gecko.
Updated•7 years ago
|
Blocks: 1368120
See Also: → https://github.com/servo/webrender/issues/1280
Comment 2•7 years ago
|
||
For the record, there are (as of right now) only 10 reftests which use hard-coded border values and would be affected by this change. layout/reftests/bugs/448193.html layout/reftests/bugs/461512-1.html layout/reftests/table-bordercollapse/frame_above_rules_none.html layout/reftests/table-bordercollapse/frame_below_rules_none.html layout/reftests/table-bordercollapse/frame_border_rules_none.html layout/reftests/table-bordercollapse/frame_box_rules_none.html layout/reftests/table-bordercollapse/frame_hsides_rules_none.html layout/reftests/table-bordercollapse/frame_lhs_rules_none.html layout/reftests/table-bordercollapse/frame_rhs_rules_none.html layout/reftests/table-bordercollapse/frame_vsides_rules_none.html
Updated•6 years ago
|
Priority: -- → P3
Updated•6 years ago
|
Blocks: webrender-reftests
Reporter | ||
Comment 3•6 years ago
|
||
Jeff, what's the process for getting a decision on this?
Flags: needinfo?(jmuizelaar)
Assignee | ||
Comment 4•6 years ago
|
||
I think we can change Gecko. Some of the table border-collapsing tests I'm fixing are going to fail because we're going to render the gecko colors for tables.
Flags: needinfo?(emilio)
Assignee | ||
Comment 5•6 years ago
|
||
See the discussion in https://github.com/servo/webrender/issues/1280. I think we should do this sooner rather than later. Need to update a couple reftests to hardcode the new colors, waiting on try for that but should be trivial. This makes a few more tests pass which are just marked as failure in bug 1487407, because I implementing the border-collapsing reusing a bunch of Gecko code, including the table 3d border stuff.
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → emilio
Status: NEW → ASSIGNED
Flags: needinfo?(jmuizelaar)
Flags: needinfo?(emilio)
Note that Chrome doesn't do what https://www.w3.org/TR/css-backgrounds-3/#the-border-style somewhat vaguely intends, at least for "groove". The Level 4 Draft https://drafts.csswg.org/css-backgrounds-4/#borders proposes multiple colors per side. Anyway, this is probably an issue for the CSS WG to specify. (The only browser doing this right was IMHO Presto-Opera, test with Opera 12.18 :-)
Assignee | ||
Comment 7•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Attachment #9011177 -
Attachment mime type: text/plain → text/html
Assignee | ||
Comment 8•6 years ago
|
||
Per the above test-case, I think we're good enough. Chrome is really bad, but presto-opera is also not great with dark backgrounds.
Comment 9•6 years ago
|
||
Comment on attachment 9011175 [details] Bug 1382896 - Align Gecko and WebRender code for 3d border colors. r=jrmuizel Markus Stange [:mstange] has approved the revision.
Attachment #9011175 -
Flags: review+
Comment 10•6 years ago
|
||
I'm a bit surprised that we're choosing to make "darkened" black brighter than undarkened black (0.3 vs 0.0), which neither Webkit nor Blink do.
Assignee | ||
Comment 11•6 years ago
|
||
Mostly straight-forward, the bordercol.css changes are just color renames, but that file had bogus newlines and such so it looks much bigger.
Comment 12•6 years ago
|
||
Comment on attachment 9011366 [details] Bug 1382896 - Update tests. Jeff Muizelaar [:jrmuizel] has approved the revision.
Attachment #9011366 -
Flags: review+
Comment 13•6 years ago
|
||
Pushed by emilio@crisal.io: https://hg.mozilla.org/integration/mozilla-inbound/rev/95dfb8f992a6 Align Gecko and WebRender code for 3d border colors. r=mstange https://hg.mozilla.org/integration/mozilla-inbound/rev/178218e7b57c Update tests. r=jrmuizel
Comment 14•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/95dfb8f992a6 https://hg.mozilla.org/mozilla-central/rev/178218e7b57c
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox64:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in
before you can comment on or make changes to this bug.
Description
•