[Webkit-unassigned] [Bug 196536] Compute accurate regions for touch-action

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=196536

Simon Fraser (smfr) <simon.fraser at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #366611|review?                     |review+
              Flags|                            |

--- Comment #10 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 366611
  --> https://bugs.webkit.org/attachment.cgi?id=366611
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=366611&action=review

> Source/WebCore/css/StyleResolver.cpp:849
> +    if (effectiveTouchActions.contains(TouchAction::None))
> +        return { TouchAction::None };

So "none" on an ancestor clobbers any value on descendants? That's weird.

>> Source/WebCore/css/StyleResolver.cpp:853
>> +        return elementTouchActions;
> 
> I don't understand the intent of this code. What is an "effective touch-action"?

I'm a bit confused by this code too. I think it needs a link to spec text.

> Source/WebCore/css/StyleResolver.cpp:1107
> +        style.setEffectiveTouchActions(computeEffectiveTouchActions(style.touchActions(), style.effectiveTouchActions()));

This might be clearer if the second param used parentStyle.effectiveTouchActions()

> Source/WebCore/rendering/PaintInfo.h:46
> +class TouchActionRegion;

Needs if ENABLE(POINTER_EVENTS) guard

> Source/WebCore/rendering/PaintInfo.h:135
> +    TouchActionRegion* touchActionRegion { nullptr };

Needs if ENABLE(POINTER_EVENTS) guard

> Source/WebCore/rendering/RenderLayer.cpp:4873
> +        paintInfo.touchActionRegion = localPaintingInfo.touchActionRegion;

if ENABLE(POINTER_EVENTS)

> Source/WebCore/rendering/RenderLayer.h:81
> +class TouchActionRegion;

if ENABLE(POINTER_EVENTS)

> Source/WebCore/rendering/RenderLayer.h:918
> +        TouchActionRegion* touchActionRegion { nullptr };

if ENABLE(POINTER_EVENTS)

> Source/WebCore/rendering/TouchActionRegion.cpp:73
> +constexpr const char* toString(TouchAction touchAction)

Or you could just implement TextStream& operator<<(TextStream&, TouchAction)

> Source/WebCore/rendering/style/RenderStyle.h:709
> +    OptionSet<TouchAction> effectiveTouchActions() const { return OptionSet<TouchAction>::fromRaw(m_rareInheritedData->effectiveTouchActions); }

I think this justifies a comment saying that "touch-action is not an inherited property, but we fake one with "effectiveTouchActions" so we can blah blah".

> Source/WebCore/rendering/style/StyleRareInheritedData.cpp:373
> +        && effectiveTouchActions == o.effectiveTouchActions

Do we want this to affect operator==? Will that trigger unwanted layouts/repaints?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190403/aff6fd90/attachment-0001.html>