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 287163010: Notify <picture> elements when a media query (potentially) changes (Closed)

Created:
6 years, 7 months ago by cbiesinger
Modified:
6 years, 5 months ago
CC:
blink-reviews, blink-reviews-dom_chromium.org, blink-reviews-html_chromium.org, dglazkov+blink, eae+blinkwatch, rwlbuis, sof
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Notify <picture> elements when a media query changes. Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=177440

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : add test #

Patch Set 4 : rebased #

Total comments: 3

Patch Set 5 : review comments #

Total comments: 7

Patch Set 6 : review comments #

Patch Set 7 : forgot to adjust test after moving #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+110 lines, -7 lines) Patch
A LayoutTests/fast/dom/HTMLImageElement/image-picture-react-to-media-changes.html View 1 2 3 4 5 6 1 chunk +36 lines, -0 lines 0 comments Download
A LayoutTests/fast/dom/HTMLImageElement/image-picture-react-to-media-changes-expected.txt View 1 2 3 4 5 6 1 chunk +6 lines, -0 lines 0 comments Download
M Source/core/css/MediaQueryListListener.h View 1 2 3 4 5 1 chunk +1 line, -1 line 1 comment Download
M Source/core/html/HTMLImageElement.cpp View 1 1 chunk +2 lines, -6 lines 0 comments Download
M Source/core/html/HTMLSourceElement.h View 1 2 3 4 5 4 chunks +11 lines, -0 lines 0 comments Download
M Source/core/html/HTMLSourceElement.cpp View 1 2 3 4 5 6 chunks +54 lines, -0 lines 0 comments Download

Messages

Total messages: 11 (0 generated)
cbiesinger
6 years, 5 months ago (2014-07-02 17:53:46 UTC) #1
Yoav Weiss
LGTM. A few nits regarding the layout test https://codereview.chromium.org/287163010/diff/60001/LayoutTests/fast/hidpi/image-srcset-react-to-media-changes.html File LayoutTests/fast/hidpi/image-srcset-react-to-media-changes.html (right): https://codereview.chromium.org/287163010/diff/60001/LayoutTests/fast/hidpi/image-srcset-react-to-media-changes.html#newcode1 LayoutTests/fast/hidpi/image-srcset-react-to-media-changes.html:1: <!DOCTYPE ...
6 years, 5 months ago (2014-07-02 18:46:05 UTC) #2
cbiesinger
Thanks, changes made. Still need an owner lgtm
6 years, 5 months ago (2014-07-02 21:00:40 UTC) #3
esprehn
https://codereview.chromium.org/287163010/diff/80001/LayoutTests/fast/dom/HTMLImageElement/image-picture-react-to-media-changes.html File LayoutTests/fast/dom/HTMLImageElement/image-picture-react-to-media-changes.html (right): https://codereview.chromium.org/287163010/diff/80001/LayoutTests/fast/dom/HTMLImageElement/image-picture-react-to-media-changes.html#newcode7 LayoutTests/fast/dom/HTMLImageElement/image-picture-react-to-media-changes.html:7: function runTest() { onload = function() { https://codereview.chromium.org/287163010/diff/80001/LayoutTests/fast/dom/HTMLImageElement/image-picture-react-to-media-changes.html#newcode36 LayoutTests/fast/dom/HTMLImageElement/image-picture-react-to-media-changes.html:36: ...
6 years, 5 months ago (2014-07-02 21:15:42 UTC) #4
sof
https://codereview.chromium.org/287163010/diff/80001/Source/core/html/HTMLSourceElement.h File Source/core/html/HTMLSourceElement.h (right): https://codereview.chromium.org/287163010/diff/80001/Source/core/html/HTMLSourceElement.h#newcode51 Source/core/html/HTMLSourceElement.h:51: HTMLSourceElement* m_element; RawPtrWillBeMember<> + add a trace() method which ...
6 years, 5 months ago (2014-07-02 21:22:18 UTC) #5
cbiesinger
All changes made! PTAL
6 years, 5 months ago (2014-07-02 21:55:18 UTC) #6
esprehn
lgtm https://codereview.chromium.org/287163010/diff/120001/Source/core/css/MediaQueryListListener.h File Source/core/css/MediaQueryListListener.h (right): https://codereview.chromium.org/287163010/diff/120001/Source/core/css/MediaQueryListListener.h#newcode56 Source/core/css/MediaQueryListListener.h:56: virtual void trace(Visitor* visitor) { visitor->trace(m_query); } OVERRIDE
6 years, 5 months ago (2014-07-02 22:40:11 UTC) #7
cbiesinger
On 2014/07/02 22:40:11, esprehn wrote: > lgtm > > https://codereview.chromium.org/287163010/diff/120001/Source/core/css/MediaQueryListListener.h > File Source/core/css/MediaQueryListListener.h (right): > ...
6 years, 5 months ago (2014-07-03 02:40:52 UTC) #8
cbiesinger
The CQ bit was checked by cbiesinger@chromium.org
6 years, 5 months ago (2014-07-03 02:41:16 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/cbiesinger@chromium.org/287163010/120001
6 years, 5 months ago (2014-07-03 02:42:47 UTC) #10
commit-bot: I haz the power
6 years, 5 months ago (2014-07-03 02:46:05 UTC) #11
Message was sent while issue was closed. 
Change committed as 177440

Powered by Google App Engine
This is Rietveld 408576698