Bug 216630 - Integrate meterElementShadow.css into html.css
Summary: Integrate meterElementShadow.css into html.css
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-09-16 18:10 PDT by Sam Weinig
Modified: 2020-09-24 11:18 PDT (History)
23 users (show)

See Also:


Attachments
Patch (18.48 KB, patch)
2020-09-16 18:19 PDT, Sam Weinig
koivisto: review-
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2020-09-16 18:10:42 PDT
Integrate meterElementShadow.css into html.css
Comment 1 Sam Weinig 2020-09-16 18:19:21 PDT
Created attachment 408974 [details]
Patch
Comment 2 Sam Weinig 2020-09-16 18:22:46 PDT
I'd like to get some feedback on this, specifically using 'class' selectors here (for .optimum, .suboptimum, and .even-less-good):

meter::-webkit-meter-value.optimum {
    background: -webkit-gradient(linear, left top, left bottom, from(#ad7), to(#ad7), color-stop(0.20, #cea), color-stop(0.45, #7a3), color-stop(0.55, #7a3));
}

meter::-webkit-meter-value.suboptimum {
    background: -webkit-gradient(linear, left top, left bottom, from(#fe7), to(#fe7), color-stop(0.20, #ffc), color-stop(0.45, #db3), color-stop(0.55, #db3));
}

meter::-webkit-meter-value.even-less-good {
    background: -webkit-gradient(linear, left top, left bottom, from(#f77), to(#f77), color-stop(0.20, #fcc), color-stop(0.45, #d44), color-stop(0.55, #d44));
}

Would it be better to define new pseudo-classes for these states (like we do elsewhere for things like autofill - :-webkit-autofill-strong-password)? What are the pros/cons of using a class vs. pseudo-class here?
Comment 3 Radar WebKit Bug Importer 2020-09-23 18:11:15 PDT
<rdar://problem/69470195>
Comment 4 Antti Koivisto 2020-09-24 10:29:29 PDT
Comment on attachment 408974 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=408974&action=review

> Source/WebCore/html/HTMLMeterElement.cpp:-232
> -    auto style = HTMLStyleElement::create(HTMLNames::styleTag, document(), false);
> -    style->setTextContent(shadowStyle);
> -    root.appendChild(style);

Why you want to undo the use of scoped style and put these things to the global stylesheet? You are essentially reverting https://bugs.webkit.org/show_bug.cgi?id=163212
Comment 5 Sam Weinig 2020-09-24 10:40:19 PDT
(In reply to Antti Koivisto from comment #4)
> Comment on attachment 408974 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=408974&action=review
> 
> > Source/WebCore/html/HTMLMeterElement.cpp:-232
> > -    auto style = HTMLStyleElement::create(HTMLNames::styleTag, document(), false);
> > -    style->setTextContent(shadowStyle);
> > -    root.appendChild(style);
> 
> Why you want to undo the use of scoped style and put these things to the
> global stylesheet? You are essentially reverting
> https://bugs.webkit.org/show_bug.cgi?id=163212

Ah, didn't know if that was a bad thing to do! I was trying to make it consistent with other similar controls (like <progress>). I was assuming this different approach was taken due to the ifdefs, but good to learn this is the future direction!

I think the same basic question is still there though, even once I move it back to scoped style. The way this currently works (I think), is that there are five pseudo-elements exposed for <meter>:

::-webkit-meter-inner-element
::-webkit-meter-bar
::-webkit-meter-optimum-value
::-webkit-meter-suboptimum-value
::-webkit-meter-even-less-good-value

The question was do those last three really make sense as pseudo-elements? or should they there be a pseudo-element called '::-webkit-meter-vale' and three pseudo-classes for 'optimum', 'suboptimum', and 'even-less-good'?
Comment 6 Antti Koivisto 2020-09-24 10:49:17 PDT
The only reason those pseudo-elements still exist is to avoid changing web-exposed behavior. The only sensible change would be to get rid of them (if we think it won't break anything that matters).

Yeah, <progress> should be implemented like this too. I did the <meter> refactoring to test the new scoping mechanism. It was then non-trivially used for media controls.
Comment 7 Sam Weinig 2020-09-24 11:18:18 PDT
Ok, cool. Will close this. We should probably separately move some of the strings I moved into NeverDestroyed, but that can be done at any point.