Hi Team, While going through 'HR' related failures on WPT, I just stumbled across following code: case AttributeNames::noshadeAttr: if (!hasAttributeWithoutSynchronization(colorAttr)) { addPropertyToPresentationalHintStyle(style, CSSPropertyBorderStyle, CSSValueSolid); auto darkGrayValue = CSSValuePool::singleton().createColorValue(Color::darkGray); style.setProperty(CSSPropertyBorderColor, darkGrayValue.ptr()); style.setProperty(CSSPropertyBackgroundColor, WTFMove(darkGrayValue)); } I noticed that we are assigning 'darkGrey' value using C++ but recently we added 'color' value from UA Stylesheet as below: https://searchfox.org/wubkat/source/Source/WebCore/css/html.css#105 ______ I looked into 'Firefox' to see if they have special color for 'noshade' attribute but they don't. Test Case to check - https://www.quanzhanketang.com/tags/tryhtml_hr_noshade.html?filename=tryhtml_hr_noshade ____ I might be completely wrong but just wanted to raise this to check, whether we can get rid of this portion of code? [These lines] auto darkGrayValue = CSSValuePool::singleton().createColorValue(Color::darkGray); style.setProperty(CSSPropertyBorderColor, darkGrayValue.ptr()); style.setProperty(CSSPropertyBackgroundColor, WTFMove(darkGrayValue)); Thanks!
Removing the darkgray override seems fine to me.
We start failing this - https://jsfiddle.net/w589b0dh/show As per spec, it is non-conforming standard with only support from Chrome and Safari only. Spec: https://html.spec.whatwg.org/multipage/obsolete.html#non-conforming-features Firefox Nightly fails a lot of tests from above. Should we keep it or remove it altogether? Or we can get rid of just this line: >> style.setProperty(CSSPropertyBorderColor, darkGrayValue.ptr()); and still make us pass above test.
(In reply to Ahmad Saleem from comment #2) > We start failing this - https://jsfiddle.net/w589b0dh/show > > As per spec, it is non-conforming standard with only support from Chrome and > Safari only. > > Spec: > https://html.spec.whatwg.org/multipage/obsolete.html#non-conforming-features > > Firefox Nightly fails a lot of tests from above. > > Should we keep it or remove it altogether? > > Or we can get rid of just this line: > > >> style.setProperty(CSSPropertyBorderColor, darkGrayValue.ptr()); > > > and still make us pass above test. I would follow the spec here: https://html.spec.whatwg.org/#the-hr-element-2 . We deviate in multiple ways from the spec: * background-color should not be set when setting the color / noshade attributes * size attribute should set the border-width to the value divided by 2 when color/noshade is set, it should set the height otherwise. More minor differences: * width should parse dimensions, not integers * size should parse non-negative integers, not all integers.
<rdar://problem/113424073>
Based on Comment 03, few changes, which work: Line 96 (Delete in 'colorAttr') - addHTMLColorToStyle(style, CSSPropertyBackgroundColor, value); Line 103 (Delete in 'noshadeAttr') - style.setProperty(CSSPropertyBackgroundColor, WTFMove(darkGrayValue)); and change Line 107 (old and now new 105) as following: if (int size = parseHTMLInteger(value).value_or(0); size > 1) to if (int size = parseHTMLNonNegativeInteger(value).value_or(0); size > 1) _____ Still figuring out 'parseDimensions' part.
(In reply to Ahmad Saleem from comment #5) > Based on Comment 03, few changes, which work: > > Line 96 (Delete in 'colorAttr') - addHTMLColorToStyle(style, > CSSPropertyBackgroundColor, value); > > Line 103 (Delete in 'noshadeAttr') - > style.setProperty(CSSPropertyBackgroundColor, WTFMove(darkGrayValue)); > > and change Line 107 (old and now new 105) as following: > > if (int size = parseHTMLInteger(value).value_or(0); size > 1) > > to > > if (int size = parseHTMLNonNegativeInteger(value).value_or(0); size > 1) > > _____ > > Still figuring out 'parseDimensions' part. There's a parseHTMLDimension method.