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

Issue 466353002: Added a console warning when <picture><source src></picture> is used. (Closed)

Created:
3 years, 3 months ago by Yoav Weiss
Modified:
3 years, 3 months ago
Reviewers:
Mike West
CC:
blink-reviews, dglazkov+blink, blink-reviews-html_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Added a console warning when <picture><source src></picture> is used. Following a discussion on the lack of support for <picture><source src></picture> that happened on https://github.com/ResponsiveImagesCG/picture-element/issues/224 I would like to add a console warning to guide authors away from this pattern. BUG=403260 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180187

Patch Set 1 #

Patch Set 2 : Making it actually work #

Patch Set 3 : Adjusted expected results #

Total comments: 1

Patch Set 4 : Added use counter #

Total comments: 1

Patch Set 5 : Added usecounter the right way #

Unified diffs Side-by-side diffs Delta from patch set Stats (+24 lines, -8 lines) Patch
A + LayoutTests/fast/dom/HTMLImageElement/image-picture-source-src.html View 1 2 3 4 1 chunk +5 lines, -4 lines 0 comments Download
A + LayoutTests/fast/dom/HTMLImageElement/image-picture-source-src-and-srcset.html View 1 2 3 4 1 chunk +5 lines, -4 lines 0 comments Download
A LayoutTests/fast/dom/HTMLImageElement/image-picture-source-src-and-srcset-expected.txt View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download
A LayoutTests/fast/dom/HTMLImageElement/image-picture-source-src-expected.txt View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download
M LayoutTests/http/tests/loading/preload-picture-sizes-2x-expected.txt View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download
M LayoutTests/http/tests/loading/preload-picture-sizes-expected.txt View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/frame/UseCounter.h View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/frame/UseCounter.cpp View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments Download
M Source/core/html/HTMLImageElement.cpp View 1 2 3 4 2 chunks +3 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
Yoav Weiss
3 years, 3 months ago (2014-08-13 10:16:15 UTC) #1
Mike West
It might be worth measuring this. LGTM either way. https://codereview.chromium.org/466353002/diff/40001/Source/core/html/HTMLImageElement.cpp File Source/core/html/HTMLImageElement.cpp (right): https://codereview.chromium.org/466353002/diff/40001/Source/core/html/HTMLImageElement.cpp#newcode287 Source/core/html/HTMLImageElement.cpp:287: ...
3 years, 3 months ago (2014-08-13 13:57:24 UTC) #2
Yoav Weiss
On 2014/08/13 13:57:24, Mike West (OOO until 19th) wrote: > It might be worth measuring ...
3 years, 3 months ago (2014-08-13 14:24:56 UTC) #3
Yoav Weiss
On 2014/08/13 14:24:56, Yoav Weiss wrote: > On 2014/08/13 13:57:24, Mike West (OOO until 19th) ...
3 years, 3 months ago (2014-08-13 14:27:16 UTC) #4
Mike West
https://codereview.chromium.org/466353002/diff/60001/Source/core/html/HTMLImageElement.cpp File Source/core/html/HTMLImageElement.cpp (right): https://codereview.chromium.org/466353002/diff/60001/Source/core/html/HTMLImageElement.cpp#newcode287 Source/core/html/HTMLImageElement.cpp:287: document().addConsoleMessage(ConsoleMessage::create(RenderingMessageSource, WarningMessageLevel, "<source src> with a <picture> parent is ...
3 years, 3 months ago (2014-08-13 14:29:31 UTC) #5
Yoav Weiss
On 2014/08/13 14:29:31, Mike West (OOO until 19th) wrote: > https://codereview.chromium.org/466353002/diff/60001/Source/core/html/HTMLImageElement.cpp > File Source/core/html/HTMLImageElement.cpp (right): ...
3 years, 3 months ago (2014-08-13 14:58:41 UTC) #6
Mike West
LGTM. On 2014/08/13 14:58:41, Yoav Weiss wrote: > Added it the right way. > One ...
3 years, 3 months ago (2014-08-13 15:01:43 UTC) #7
Yoav Weiss
The CQ bit was checked by yoav@yoav.ws
3 years, 3 months ago (2014-08-13 15:46:58 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yoav@yoav.ws/466353002/80001
3 years, 3 months ago (2014-08-13 15:48:14 UTC) #9
commit-bot: I haz the power
3 years, 3 months ago (2014-08-13 16:15:52 UTC) #10
Message was sent while issue was closed.
Committed patchset #5 (80001) as 180187

Powered by Google App Engine
This is Rietveld efc10ee0f