Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(765)

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

Created:
3 years, 7 months ago by alancutter (OOO until 2018)
Modified:
3 years, 7 months 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)
3 years, 7 months 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 ...
3 years, 7 months 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 ...
3 years, 7 months 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 ...
3 years, 7 months 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
3 years, 7 months 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)
3 years, 7 months 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
3 years, 7 months 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)
3 years, 7 months 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
3 years, 7 months 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)
3 years, 7 months 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
3 years, 7 months ago (2017-05-12 02:38:07 UTC) #23
commit-bot: I haz the power
3 years, 7 months 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 408576698