Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(3)

Issue 2873943002: Ensure original parser context is used when parsing resolved var() references (Closed)

Created:
6 months, 2 weeks ago by alancutter (OOO until 2018)
Modified:
6 months, 1 week ago
Reviewers:
Timothy Loh
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, chromium-reviews, dglazkov+blink, rwlbuis
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Ensure original parser context is used when parsing resolved var() references When resolving var() references at style building time we were not using the CSSParserContext specific to where the var() came from. This resulted in relative url()s not being able to resolve to an appropriate absolute URL based on the source base URL. This change attaches CSSParserContexts to var() references so they can be used at style resolution time and appropriately resolve relative URLs. BUG=700445 Review-Url: https://codereview.chromium.org/2873943002 Cr-Commit-Position: refs/heads/master@{#471205} Committed: https://chromium.googlesource.com/chromium/src/+/a3079e2f1ea9974caebcdebd00c1c33dbaa662e0

Patch Set 1 #

Patch Set 2 : Moar asserts #

Patch Set 3 : CSSParserContext member #

Patch Set 4 : testcomment #

Total comments: 1

Patch Set 5 : g cl set-commit #

Unified diffs Side-by-side diffs Delta from patch set Stats (+127 lines, -14 lines) Patch
A third_party/WebKit/LayoutTests/fast/css/resources/image-url-var.css View 1 2 1 chunk +31 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/css/variables/computed-image-url.html View 1 1 chunk +26 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/css/variables/computed-stylesheet-image-url.html View 1 2 3 1 chunk +35 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSSyntaxDescriptor.cpp View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSVariableReferenceValue.h View 1 2 2 chunks +15 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSVariableReferenceValue.cpp View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/cssom/CSSUnparsedValue.cpp View 1 2 3 4 1 chunk +7 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/css/parser/CSSVariableParser.h View 1 2 2 chunks +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/parser/CSSVariableParser.cpp View 1 2 2 chunks +3 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/css/resolver/CSSVariableResolver.cpp View 1 2 2 chunks +3 lines, -3 lines 0 comments Download

Messages

Total messages: 26 (14 generated)
alancutter (OOO until 2018)
6 months, 2 weeks ago (2017-05-10 07:27:52 UTC) #2
Timothy Loh
On 2017/05/10 07:27:52, alancutter wrote: Don't think this does the right thing for stylesheets loaded ...
6 months, 2 weeks ago (2017-05-10 07:30:07 UTC) #3
alancutter (OOO until 2018)
On 2017/05/10 at 07:30:07, timloh wrote: > On 2017/05/10 07:27:52, alancutter wrote: > > Don't ...
6 months, 1 week ago (2017-05-11 03:40:24 UTC) #5
Timothy Loh
lgtm https://codereview.chromium.org/2873943002/diff/60001/third_party/WebKit/Source/core/css/cssom/CSSUnparsedValue.cpp File third_party/WebKit/Source/core/css/cssom/CSSUnparsedValue.cpp (right): https://codereview.chromium.org/2873943002/diff/60001/third_party/WebKit/Source/core/css/cssom/CSSUnparsedValue.cpp#newcode94 third_party/WebKit/Source/core/css/cssom/CSSUnparsedValue.cpp:94: *StrictCSSParserContext()); Add a TODO, this probably needs a ...
6 months, 1 week ago (2017-05-11 04:56:18 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2873943002/80001
6 months, 1 week ago (2017-05-11 05:41:53 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/441624)
6 months, 1 week ago (2017-05-11 07:30:45 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2873943002/80001
6 months, 1 week ago (2017-05-11 07:32:56 UTC) #15
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/441709)
6 months, 1 week ago (2017-05-11 08:05:50 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2873943002/80001
6 months, 1 week ago (2017-05-11 12:49:22 UTC) #19
commit-bot: I haz the power
Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_swarming_rel/builds/175890)
6 months, 1 week ago (2017-05-11 14:53:28 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2873943002/80001
6 months, 1 week ago (2017-05-12 02:38:07 UTC) #23
commit-bot: I haz the power
6 months, 1 week ago (2017-05-12 03:18:32 UTC) #26
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/a3079e2f1ea9974caebcdebd00c1...

Powered by Google App Engine
This is Rietveld efc10ee0f