Bug 158449 - Writing-mode-dependent properties don't apply if their value is a variable
Summary: Writing-mode-dependent properties don't apply if their value is a variable
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
Keywords: InRadar
Depends on:
Reported: 2016-06-06 16:53 PDT by Tim Horton
Modified: 2016-06-09 11:31 PDT (History)
8 users (show)

See Also:

repro (584 bytes, text/html)
2016-06-06 16:53 PDT, Tim Horton
no flags Details
Patch (7.35 KB, patch)
2016-06-09 10:53 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (7.39 KB, patch)
2016-06-09 11:00 PDT, Tim Horton
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2016-06-06 16:53:54 PDT
Created attachment 280647 [details]

The two boxes in the attached test case should render identically.

However, the box which has -webkit-padding-start: var(--margin-leading); ends up getting no horizontal padding, because referencing variables in writing-mode/direction-dependent properties does not work.
Comment 1 Tim Horton 2016-06-06 16:54:21 PDT
I did some debugging, and found the point where this goes wrong, but I’m not sure how to fix it.

Once we get into CSSParser::parseVariableDependentValue, our propID is the resolved one (PaddingLeft, for example). The CSSVariableDependentValue still has the unresolved propID (WebkitPaddingStart), though, so the comparison (property.id() == propID) fails, and we end up returning nullptr.

I’m not sure if/when we should be doing the resolution, though. Inside parseVariableDependentValue?
Comment 2 Radar WebKit Bug Importer 2016-06-06 16:55:29 PDT
Comment 3 Radar WebKit Bug Importer 2016-06-06 16:55:52 PDT
Comment 4 Tim Horton 2016-06-09 10:53:23 PDT
Created attachment 280928 [details]
Comment 5 Simon Fraser (smfr) 2016-06-09 10:55:05 PDT
Comment on attachment 280928 [details]

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

> Source/WebCore/css/CSSParser.cpp:1871
>      for (auto& property : m_parsedProperties) {

I would prefer a blank line above this one.
Comment 6 Tim Horton 2016-06-09 11:00:03 PDT
Created attachment 280929 [details]
Comment 7 WebKit Commit Bot 2016-06-09 11:31:32 PDT
Comment on attachment 280929 [details]

Clearing flags on attachment: 280929

Committed r201875: <http://trac.webkit.org/changeset/201875>
Comment 8 WebKit Commit Bot 2016-06-09 11:31:38 PDT
All reviewed patches have been landed.  Closing bug.