Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2863603002:
Change Monochrome media query evaluator (Closed)
DescriptionChange Monochrome media query evaluator
Change this to match ColorMediaFeatureEval's behavior.
Previously, this was incorrectly comparing the monochrome
feature against color bit depth.
BUG=645697
Review-Url: https://codereview.chromium.org/2863603002
Cr-Commit-Position: refs/heads/master@{#469478}
Committed: https://chromium.googlesource.com/chromium/src/+/c9ef836bccf4a19fde92b42f6d68e8895f0a1a49
Patch Set 1 #Patch Set 2 : Add test #
Total comments: 1
Messages
Total messages: 22 (14 generated)
The CQ bit was checked by ccameron@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
ccameron@chromium.org changed reviewers: + esprehn@chromium.org
I have absolutely no idea what this code does, but it fixes https://jsfiddle.net/sytuqzdw/1/ Maybe it also breaks some things?
rune@opera.com changed reviewers: + rune@opera.com
You need to add some tests.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Added a test
The CQ bit was checked by ccameron@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2863603002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/css/MediaQueryEvaluator.cpp (right): https://codereview.chromium.org/2863603002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/css/MediaQueryEvaluator.cpp:238: float number; Nit: I think I'd put this declaration inside the if-block.
On 2017/05/04 20:52:37, rune wrote: > lgtm > > https://codereview.chromium.org/2863603002/diff/20001/third_party/WebKit/Sour... > File third_party/WebKit/Source/core/css/MediaQueryEvaluator.cpp (right): > > https://codereview.chromium.org/2863603002/diff/20001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/css/MediaQueryEvaluator.cpp:238: float number; > Nit: I think I'd put this declaration inside the if-block. Could you also try to explain the fix in more detail in the description? Say that we were incorrectly comparing the monochrome feature against color bit depth?
Description was changed from ========== Change Monochrome media query evaluator Change this to match ColorMediaFeatureEval's behavior. BUG=645697 ========== to ========== Change Monochrome media query evaluator Change this to match ColorMediaFeatureEval's behavior. Previously, this was incorrectly comparing the monochrome feature against color bit depth. BUG=645697 ==========
Thanks!
The CQ bit was checked by ccameron@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493932680911310,
"parent_rev": "d01fb51d1a9ba850c92f525dc31187ef79a2586c", "commit_rev":
"c9ef836bccf4a19fde92b42f6d68e8895f0a1a49"}
Message was sent while issue was closed.
Description was changed from ========== Change Monochrome media query evaluator Change this to match ColorMediaFeatureEval's behavior. Previously, this was incorrectly comparing the monochrome feature against color bit depth. BUG=645697 ========== to ========== Change Monochrome media query evaluator Change this to match ColorMediaFeatureEval's behavior. Previously, this was incorrectly comparing the monochrome feature against color bit depth. BUG=645697 Review-Url: https://codereview.chromium.org/2863603002 Cr-Commit-Position: refs/heads/master@{#469478} Committed: https://chromium.googlesource.com/chromium/src/+/c9ef836bccf4a19fde92b42f6d68... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/c9ef836bccf4a19fde92b42f6d68... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
