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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2718273002: Resolve background image's url if absolute url is empty (Closed)

Created:
3 years, 9 months ago by Sunghoon Kim
Modified:
3 years, 9 months ago
CC:
chromium-reviews, blink-reviews-css, dglazkov+blink, apavlov+blink_chromium.org, darktears, blink-reviews, rwlbuis
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Resolve background image's url if absolute url is empty Background image's url in iframe is not resolved when CSSImageValue is created, thus absolute url becomes empty. Before requesting image with absolute url, do resolving absolute url if it is empty. BUG=270059 Review-Url: https://codereview.chromium.org/2718273002 Cr-Commit-Position: refs/heads/master@{#454173} Committed: https://chromium.googlesource.com/chromium/src/+/7b66b5aeec98d9106b6e861a718a7855daf39c08

Patch Set 1 #

Total comments: 4

Patch Set 2 : Resolve background image url before use #

Patch Set 3 : Resolve background image url before use #

Total comments: 1

Patch Set 4 : Resolve background image url before use #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -0 lines) Patch
A third_party/WebKit/LayoutTests/fast/backgrounds/background-image-relative-url-in-iframe.html View 1 1 chunk +14 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/backgrounds/background-image-relative-url-in-iframe-expected.html View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSImageValue.cpp View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 12 (5 generated)
Sunghoon Kim
3 years, 9 months ago (2017-02-28 01:05:23 UTC) #2
rune
https://codereview.chromium.org/2718273002/diff/1/third_party/WebKit/LayoutTests/fast/backgrounds/background-image-relative-url-in-iframe-expected.html File third_party/WebKit/LayoutTests/fast/backgrounds/background-image-relative-url-in-iframe-expected.html (right): https://codereview.chromium.org/2718273002/diff/1/third_party/WebKit/LayoutTests/fast/backgrounds/background-image-relative-url-in-iframe-expected.html#newcode21 third_party/WebKit/LayoutTests/fast/backgrounds/background-image-relative-url-in-iframe-expected.html:21: </body> If you set the background on the iframe ...
3 years, 9 months ago (2017-02-28 09:29:06 UTC) #3
Sunghoon Kim
Thank you for your kind review. I modified as your comment and passed layout test.
3 years, 9 months ago (2017-03-01 23:40:17 UTC) #4
rune
lgtm with nit https://codereview.chromium.org/2718273002/diff/40001/third_party/WebKit/LayoutTests/fast/backgrounds/background-image-relative-url-in-iframe-expected.html File third_party/WebKit/LayoutTests/fast/backgrounds/background-image-relative-url-in-iframe-expected.html (right): https://codereview.chromium.org/2718273002/diff/40001/third_party/WebKit/LayoutTests/fast/backgrounds/background-image-relative-url-in-iframe-expected.html#newcode3 third_party/WebKit/LayoutTests/fast/backgrounds/background-image-relative-url-in-iframe-expected.html:3: <body> Just skip html and body ...
3 years, 9 months ago (2017-03-02 00:22:39 UTC) #5
Sunghoon Kim
Thank you for your review.
3 years, 9 months ago (2017-03-02 01:11:06 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/2718273002/60001
3 years, 9 months ago (2017-03-02 01:11:13 UTC) #9
commit-bot: I haz the power
3 years, 9 months ago (2017-03-02 03:51:43 UTC) #12
Message was sent while issue was closed. 
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/7b66b5aeec98d9106b6e861a718a...

Powered by Google App Engine
This is Rietveld 408576698