Move touch-action property into CSSPropertyParser

Move touch-action property into CSSPropertyParser

Move touch-action property handling from LegacyCSSPropertyParser
into CSSPropertyParser.

BUG=499780
Committed: https://crrev.com/5083511929d92372dae52be569ba6188b766e1cb
Cr-Commit-Position: refs/heads/master@{#354296}

[rwlbuis, 2015-10-14 23:00:36]

rob.buis@samsung.com changed reviewers:
+ timloh@chromium.org

[rwlbuis, 2015-10-14 23:00:36]

PTAL.

[Timothy Loh, 2015-10-14 23:45:45]

https://codereview.chromium.org/1406763002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/1406763002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:841: if (id ==
CSSValuePanX || id == CSSValuePanRight || id == CSSValuePanLeft) {
Should we check if panX is null here (similar for panY)? I assume pan-x pan-x is
invalid.

[rwlbuis, 2015-10-14 23:54:54]

https://codereview.chromium.org/1406763002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/1406763002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:841: if (id ==
CSSValuePanX || id == CSSValuePanRight || id == CSSValuePanLeft) {
On 2015/10/14 23:45:45, Timothy Loh wrote:
> Should we check if panX is null here (similar for panY)? I assume pan-x pan-x
is
> invalid.

Yes, I think that is invalid. Below panX check should catch it and make it
impossible, I think there is a layout test for it that we do pass.

[rwlbuis, 2015-10-14 23:58:06]

https://codereview.chromium.org/1406763002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/1406763002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:841: if (id ==
CSSValuePanX || id == CSSValuePanRight || id == CSSValuePanLeft) {
On 2015/10/14 23:54:54, rwlbuis wrote:
> On 2015/10/14 23:45:45, Timothy Loh wrote:
> > Should we check if panX is null here (similar for panY)? I assume pan-x
pan-x
> is
> > invalid.
> 
> Yes, I think that is invalid. Below panX check should catch it and make it
> impossible, I think there is a layout test for it that we do pass.

Ah, you mean add the check to the outer if(s) instead? That would have the same
effect but may be a bit easier to read indeed.

[Timothy Loh, 2015-10-14 23:59:01]

lgtm

https://codereview.chromium.org/1406763002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/1406763002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:841: if (id ==
CSSValuePanX || id == CSSValuePanRight || id == CSSValuePanLeft) {
On 2015/10/14 23:58:06, rwlbuis wrote:
> On 2015/10/14 23:54:54, rwlbuis wrote:
> > On 2015/10/14 23:45:45, Timothy Loh wrote:
> > > Should we check if panX is null here (similar for panY)? I assume pan-x
> pan-x
> > is
> > > invalid.
> > 
> > Yes, I think that is invalid. Below panX check should catch it and make it
> > impossible, I think there is a layout test for it that we do pass.
> 
> Ah, you mean add the check to the outer if(s) instead? That would have the
same
> effect but may be a bit easier to read indeed.

I actually didn't see it in the below if statement ;), I think at the start of
the outer ifs would be better, yeah.

[rwlbuis, 2015-10-15 00:00:47]

On 2015/10/14 23:59:01, Timothy Loh wrote:
> lgtm
> 
>
https://codereview.chromium.org/1406763002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):
> 
>
https://codereview.chromium.org/1406763002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:841: if (id ==
> CSSValuePanX || id == CSSValuePanRight || id == CSSValuePanLeft) {
> On 2015/10/14 23:58:06, rwlbuis wrote:
> > On 2015/10/14 23:54:54, rwlbuis wrote:
> > > On 2015/10/14 23:45:45, Timothy Loh wrote:
> > > > Should we check if panX is null here (similar for panY)? I assume pan-x
> > pan-x
> > > is
> > > > invalid.
> > > 
> > > Yes, I think that is invalid. Below panX check should catch it and make it
> > > impossible, I think there is a layout test for it that we do pass.
> > 
> > Ah, you mean add the check to the outer if(s) instead? That would have the
> same
> > effect but may be a bit easier to read indeed.
> 
> I actually didn't see it in the below if statement ;), I think at the start of
> the outer ifs would be better, yeah.

Sure, will fix, thanks!

[rwlbuis, 2015-10-15 14:02:41]

The CQ bit was checked by rob.buis@samsung.com

[rwlbuis, 2015-10-15 14:02:41]

The patchset sent to the CQ was uploaded after l-g-t-m from timloh@chromium.org
Link to the patchset: https://codereview.chromium.org/1406763002/#ps40001
(title: "Patch for landing")

[commit-bot: I haz the power, 2015-10-15 14:04:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1406763002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1406763002/40001

[rwlbuis, 2015-10-15 14:08:52]

Patchset #4 (id:60001) has been deleted

[rwlbuis, 2015-10-15 16:49:07]

The CQ bit was checked by rob.buis@samsung.com

[rwlbuis, 2015-10-15 16:49:07]

The patchset sent to the CQ was uploaded after l-g-t-m from timloh@chromium.org
Link to the patchset: https://codereview.chromium.org/1406763002/#ps40001
(title: "Patch for landing")

[commit-bot: I haz the power, 2015-10-15 16:49:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1406763002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1406763002/40001

[commit-bot: I haz the power, 2015-10-15 17:44:54]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-10-15 17:45:54]

Patchset 3 (id:??) landed as
https://crrev.com/5083511929d92372dae52be569ba6188b766e1cb
Cr-Commit-Position: refs/heads/master@{#354296}