Chromium Code Reviews
Help | Chromium Project | Sign in
(179)

Issue 287163010: Notify <picture> elements when a media query (potentially) changes (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 2 months ago by cbiesinger
Modified:
1 year, 1 month 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

Messages

Total messages: 11 (0 generated)
cbiesinger
1 year, 1 month ago (2014-07-02 17:53:46 UTC) #1
Yoav Weiss (OOO until 15th)
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 ...
1 year, 1 month ago (2014-07-02 18:46:05 UTC) #2
cbiesinger
Thanks, changes made. Still need an owner lgtm
1 year, 1 month ago (2014-07-02 21:00:40 UTC) #3
esprehn (ooo-until-aug-10)
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: ...
1 year, 1 month 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 ...
1 year, 1 month ago (2014-07-02 21:22:18 UTC) #5
cbiesinger
All changes made! PTAL
1 year, 1 month ago (2014-07-02 21:55:18 UTC) #6
esprehn (ooo-until-aug-10)
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
1 year, 1 month 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): > ...
1 year, 1 month ago (2014-07-03 02:40:52 UTC) #8
cbiesinger
The CQ bit was checked by cbiesinger@chromium.org
1 year, 1 month 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
1 year, 1 month ago (2014-07-03 02:42:47 UTC) #10
commit-bot: I haz the power
1 year, 1 month ago (2014-07-03 02:46:05 UTC) #11
Message was sent while issue was closed.
Change committed as 177440
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 3ea459f