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)

enhancement

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.
Duplicate of bug 50699, no?
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
Priority: -- → P3
Jeff, what's the process for getting a decision on this?
Flags: needinfo?(jmuizelaar)
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)
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: 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: emilio → nobody
Blocks: 50699
Status: ASSIGNED → NEW
Assignee: nobody → emilio
Status: NEW → ASSIGNED
Attachment #9011177 - Attachment mime type: text/plain → text/html
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 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+
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.
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 on attachment 9011366 [details]
Bug 1382896 - Update tests.

Jeff Muizelaar [:jrmuizel] has approved the revision.
Attachment #9011366 - Flags: review+
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
https://hg.mozilla.org/mozilla-central/rev/95dfb8f992a6
https://hg.mozilla.org/mozilla-central/rev/178218e7b57c
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Depends on: 1494519
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: