New issue
Advanced search Search tips

Issue 803699 link

Starred by 3 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

-webkit-app-region CSS implementation is not self-consistent

Project Member Reported by ericwilligers@chromium.org, Jan 19 2018

Issue description

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/CSSProperties.json5?type=cs&q=none&sq=package:chromium&l=2870
says there are three valid keywords:
none, drag, no-drag

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/LayoutInline.cpp?q=DraggableRegionMode&sq=package:chromium&l=1583&dr=C
has different logic for all three states during layout.


However, the parser and serializer only respect two CSS identifiers: 'drag' and 'no-drag'?
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/parser/CSSParserFastPaths.cpp?q=CSSValueDrag&sq=package:chromium&l=742&dr=C
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/properties/longhands/WebkitAppRegionCustom.cpp?q=CSSValueDrag&sq=package:chromium&l=21&dr=C

We seem to rely on the computed style constructor as the only way to set the value to none.

Perhaps 'none' should also be accepted by the parser and serializer and applyValueCSSPropertyWebkitAppRegion?





https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp?q=webkitAppRegion&sq=package:chromium&l=807&dr=CSs
applyInitialCSSPropertyWebkitAppRegion() and applyInheritCSSPropertyWebkitAppRegion() currently do nothing.

Should applyInitialCSSPropertyWebkitAppRegion() call
state.Style()->SetDraggableRegionMode(EDraggableRegionMode::kNone) ?

Should applyInheritCSSPropertyWebkitAppRegion()
read the parent's DraggableRegionMode()?


 
Owner: rob.b...@samsung.com
Status: Assigned (was: Available)
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/766bc54b4093641aa646a908b9c586e0898993af

commit 766bc54b4093641aa646a908b9c586e0898993af
Author: Rob Buis <rob.buis@samsung.com>
Date: Fri Mar 16 12:25:26 2018

Make -webkit-app-region CSS implementation self-consistent

The property -webkit-app-region went through renames and other changes.
It seems we can settle on none | drag | no-drag. The value none is used
internally as default, make it explicit by allowing it to be specified
in the style.

Bug: 803699

Change-Id: I1ec96c9e436c7595ac7ab34d90bfbfdd9ea48672
Reviewed-on: https://chromium-review.googlesource.com/964381
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Commit-Queue: Rob Buis <rob.buis@samsung.com>
Cr-Commit-Position: refs/heads/master@{#543671}
[modify] https://crrev.com/766bc54b4093641aa646a908b9c586e0898993af/third_party/WebKit/LayoutTests/fast/css/getComputedStyle/computed-style-listing-expected.txt
[modify] https://crrev.com/766bc54b4093641aa646a908b9c586e0898993af/third_party/WebKit/LayoutTests/fast/css/getComputedStyle/computed-style-without-renderer-listing-expected.txt
[modify] https://crrev.com/766bc54b4093641aa646a908b9c586e0898993af/third_party/WebKit/LayoutTests/svg/css/getComputedStyle-listing-expected.txt
[modify] https://crrev.com/766bc54b4093641aa646a908b9c586e0898993af/third_party/WebKit/Source/core/css/parser/CSSParserFastPaths.cpp
[modify] https://crrev.com/766bc54b4093641aa646a908b9c586e0898993af/third_party/WebKit/Source/core/css/properties/longhands/WebkitAppRegionCustom.cpp

Eric, do you think there is anything more to to for this bug?
The attached example illustrates the need for applyInitialCSSPropertyWebkitAppRegion and applyInheritCSSPropertyWebkitAppRegion to be implemented.

And/or there might be a bug elsewhere.

break-after works as expected, -webkit-app-region does not.

803699_example.html
2.3 KB View Download

Comment 5 by e...@chromium.org, Apr 23 2018

Owner: ----
Status: Available (was: Assigned)

Sign in to add a comment