1382896 - Consider simplifying groove / ridge / inset / outset border rendering to match Chrome.

Status: RESOLVED FIXED

(Core :: Web Painting, enhancement, P3)

Description

[Glenn Watson [:gw]]

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.

Comment 1

[j.j.]

Duplicate of bug 50699, no?

Comment 2

[Kartikaya Gupta (email:[e-mail])]

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

Comment 3

[Glenn Watson [:gw]]

Jeff, what's the process for getting a decision on this?

Comment 4

[Emilio Cobos Álvarez [:emilio]]

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.

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1382896 - Align Gecko and WebRender code for 3d border colors. r=jrmuizel

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.

Comment 6

[j.j.]

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 :-)

Comment 7

[Emilio Cobos Álvarez [:emilio]]

Attachment: Test page taken from the GitHub issue.

Comment 8

[Emilio Cobos Álvarez [:emilio]]

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

[Markus Stange [:mstange]]

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.

Comment 10

[Markus Stange [:mstange]]

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.

Comment 11

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1382896 - Update tests.

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

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 9011366 [details]
Bug 1382896 - Update tests.

Jeff Muizelaar [:jrmuizel] has approved the revision.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
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

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/95dfb8f992a6
https://hg.mozilla.org/mozilla-central/rev/178218e7b57c