Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<dialog> UA styles are not interoperable, and should use system colors. #7754

Closed
emilio opened this issue Mar 27, 2022 · 10 comments · Fixed by #7755
Closed

<dialog> UA styles are not interoperable, and should use system colors. #7754

emilio opened this issue Mar 27, 2022 · 10 comments · Fixed by #7755
Labels
agenda+ To be discussed at a triage meeting topic: dialog The <dialog> element. topic: rendering

Comments

@emilio
Copy link
Contributor

emilio commented Mar 27, 2022

https://html.spec.whatwg.org/#flow-content-3 has:

dialog {
  background: white;
  color: black;
}

That's what Firefox implements to the letter, but:

To fix the above two things, I think we should instead make those two lines:

background-color: Canvas;
color: CanvasText;

(background-color instead of background because it's cheaper and shouldn't be observable, but...)

cc @nt1m @Futhark

@emilio emilio added topic: rendering topic: dialog The <dialog> element. agenda+ To be discussed at a triage meeting labels Mar 27, 2022
@emilio
Copy link
Contributor Author

emilio commented Mar 27, 2022

Err, cc @lilles, sorry for the needless ping at-futhark :)

@emilio
Copy link
Contributor Author

emilio commented Mar 27, 2022

@emilio
Copy link
Contributor Author

emilio commented Mar 27, 2022

https://bugzilla.mozilla.org/show_bug.cgi?id=1761611 has tentative patch and tests assuming this won't be too controversial.

@nt1m
Copy link
Member

nt1m commented Mar 27, 2022

Yeah, I agree with this, I was implementing the spec to the letter. This is something I wanted to bring up after I noticed Chromium used -internal-light-dark(...).

+1 for Canvas/CanvasText combo.

@nt1m
Copy link
Member

nt1m commented Mar 27, 2022

I filed https://bugs.webkit.org/show_bug.cgi?id=238425, will post a patch when WPT is upstream.

emilio added a commit to emilio/html that referenced this issue Mar 27, 2022
@lilles
Copy link
Contributor

lilles commented Mar 28, 2022

Totally agree canvas/canvastext makes sense.

@mfreed7

@lilles
Copy link
Contributor

lilles commented Mar 28, 2022

domenic pushed a commit that referenced this issue Mar 28, 2022
@domenic
Copy link
Member

domenic commented Mar 28, 2022

Given how fast people converged here, the same people may be interested in #5426 , which proposes making changes to other hard-coded colors.

The latest list of to-dos was in #5426 (comment) . Although, looking at how this was resolved, it seems like maybe my step (1) there is not a blocker? After that the thread got hijacked by the color-contrast debate that seems to be raging across multiple repositories... I'm hopeful some strong implementer involvement could get things back on track.

@emilio
Copy link
Contributor Author

emilio commented Mar 28, 2022

Fwiw i don't think we want to hardcode the specific colors. For example in Firefox link colors are configurable. We already use system colors for those, which react to dark mode with hopefully enough contrast.

moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 28, 2022
To be landed when whatwg/html#7754 has a resolution,
but I think it should be uncontroversial.

Differential Revision: https://phabricator.services.mozilla.com/D142170

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1761611
gecko-commit: 8f2f77d1e339d6d0c0a4e1acb3834ff87e1444c0
gecko-reviewers: morgan
aarongable pushed a commit to chromium/chromium that referenced this issue Mar 29, 2022
Using system colors improves the behavior for forced colors.

whatwg/html#7754

Bug: 1310742
Change-Id: I0d0ce6519efe3b642f26e6a27deb68d302facfcc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3550973
Reviewed-by: Mason Freed <masonf@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/main@{#986392}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Mar 29, 2022
To be landed when whatwg/html#7754 has a resolution,
but I think it should be uncontroversial.

Differential Revision: https://phabricator.services.mozilla.com/D142170
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 29, 2022
To be landed when whatwg/html#7754 has a resolution,
but I think it should be uncontroversial.

Differential Revision: https://phabricator.services.mozilla.com/D142170

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1761611
gecko-commit: 8f2f77d1e339d6d0c0a4e1acb3834ff87e1444c0
gecko-reviewers: morgan
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Mar 29, 2022
To be landed when whatwg/html#7754 has a resolution,
but I think it should be uncontroversial.

Differential Revision: https://phabricator.services.mozilla.com/D142170
@annevk
Copy link
Member

annevk commented Apr 4, 2022

@emilio right, the problem is that they are hardcoded in the HTML Standard currently.

mfreed7 pushed a commit to mfreed7/html that referenced this issue Jun 3, 2022
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Using system colors improves the behavior for forced colors.

whatwg/html#7754

Bug: 1310742
Change-Id: I0d0ce6519efe3b642f26e6a27deb68d302facfcc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3550973
Reviewed-by: Mason Freed <masonf@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/main@{#986392}
NOKEYCHECK=True
GitOrigin-RevId: 2d8a17550f1ba47984c62b3216468f027eaabd14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agenda+ To be discussed at a triage meeting topic: dialog The <dialog> element. topic: rendering
Development

Successfully merging a pull request may close this issue.

5 participants