Bug 256040 – Do not parse caption-side: left / right as valid CSS

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Ahmad Saleem]

Hi Team,

Currently we are failing following WPT test (two sub-tests) in STP168 & WebKit ToT:

Test Case View - https://wpt.fyi/results/css/css-tables/parsing/caption-side-invalid.html?label=master&label=experimental&aligned=&q=css-tables%2F

Test Case Link - http://wpt.live/css/css-tables/parsing/caption-side-invalid.html

Blink fixed it via following commit - https://chromium.googlesource.com/chromium/src.git/+/4af5b86049ac7e3b8fb3db9b5cb9383386dd7bbe

Just wanted to raise so we can track and fix it and it might be quick win.

Thanks!

[Ahmad Saleem]

I tried to find for cases in 'CSSPropertyCaptionSide' but couldn't find except this:

https://searchfox.org/wubkat/source/Source/WebCore/css/parser/CSSPropertyParser.cpp#1139

and then this:

https://searchfox.org/wubkat/source/Source/WebCore/css/CSSProperties.json#3071

______

Let me try to remove 'left' and 'right' from .json file from above and see if it works.

Although, if we need to remove elsewhere as well, appreciate if someone else can guide. Thanks!

[Ahmad Saleem]

(In reply to Ahmad Saleem from comment #1)
> I tried to find for cases in 'CSSPropertyCaptionSide' but couldn't find
> except this:
> 
> https://searchfox.org/wubkat/source/Source/WebCore/css/parser/
> CSSPropertyParser.cpp#1139
> 
> and then this:
> 
> https://searchfox.org/wubkat/source/Source/WebCore/css/CSSProperties.
> json#3071
> 
> ______
> 
> Let me try to remove 'left' and 'right' from .json file from above and see
> if it works.
> 
> Although, if we need to remove elsewhere as well, appreciate if someone else
> can guide. Thanks!

Removing just from CSSProperties.json makes us pass this bit.

[Karl Dubost]

I see mentions of left/right associated to captionside in 

https://searchfox.org/wubkat/rev/36b39fa88451d43c2afb6996534a13381260008e/Source/WebCore/rendering/style/RenderStyleConstants.h#751-756

https://searchfox.org/wubkat/rev/36b39fa88451d43c2afb6996534a13381260008e/Source/WebCore/css/CSSPrimitiveValueMappings.h#561-565

https://searchfox.org/wubkat/rev/36b39fa88451d43c2afb6996534a13381260008e/Source/WebCore/rendering/style/RenderStyleConstants.cpp#222-231


but captionside was added by Sam Weinig very early in 2008.

[Karl Dubost]

That said, I wonder about left and right because of the spec:

> These two values are added only for implementations that support left and right values for caption-side. The left and right values themselves are defined to be line-relative.
— https://w3c.github.io/csswg-drafts/css-logical/#caption-side


The two values being `inline-start` and `inline-end`.


> line-relative
> Interpreted relative to the orientation of the line box. The line-relative directions are line-left, line-right, line-over, and line-under.
— https://w3c.github.io/csswg-drafts/css-writing-modes-4/#line-relative

[Radar WebKit Bug Importer]

<rdar://problem/108892083>

[Ahmad Saleem]

(In reply to Ahmad Saleem from comment #2)
> (In reply to Ahmad Saleem from comment #1)
> > I tried to find for cases in 'CSSPropertyCaptionSide' but couldn't find
> > except this:
> > 
> > https://searchfox.org/wubkat/source/Source/WebCore/css/parser/
> > CSSPropertyParser.cpp#1139
> > 
> > and then this:
> > 
> > https://searchfox.org/wubkat/source/Source/WebCore/css/CSSProperties.
> > json#3071
> > 
> > ______
> > 
> > Let me try to remove 'left' and 'right' from .json file from above and see
> > if it works.
> > 
> > Although, if we need to remove elsewhere as well, appreciate if someone else
> > can guide. Thanks!
> 
> Removing just from CSSProperties.json makes us pass this bit.

Plus I run following:

Tools/Scripts/run-webkit-tests fast/table imported/w3c/web-platform-tests/css/ imported/w3c/web-platform-tests/html/semantics


to see, if we fail any 'caption' tests except just two progressions. I think we don't have any regressions or failure but also to highlight, we haven't imported 'css-table' from the WPT yet (which is bug 252516). So we don't know full breakage as well but just wanted to give what I have tried so far on this bug.

[Karl Dubost]

Let's hope that it will be useful to solve this issue. :) 
https://github.com/WebKit/WebKit/pull/13920

[Karl Dubost]

Ahmad, 
/css/css-tables are imported.
Bug bug 252516 is closed.

[Ahmad Saleem]

(In reply to Karl Dubost from comment #8)
> Ahmad, 
> /css/css-tables are imported.
> Bug bug 252516 is closed.

Thanks @Karl. Appreciate it.

PR - https://github.com/WebKit/WebKit/pull/14069

[EWS]

Committed 264262@main (402ed699af6c): <https://commits.webkit.org/264262@main>

Reviewed commits have been landed. Closing PR #14069 and removing active labels.