Bug 84571 – [BlackBerry] Input Range element expects mouse events

• Component: WebKit BlackBerry
• Status: RESOLVED FIXED

[Charles Wei]

Some shadow elements expects the mouse events to operate on them to change their state/value,  like SliderThumbElement, MediaControlTimelineElement.
If we don't convert touch events to mouse events for them,  then they won't work when touch and move on them.

[Charles Wei]

Created attachment 138303 [details]
Patch

[Antonio Gomes]

Comment on attachment 138303 [details]
Patch

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

It needs more info, and perhaps more discussion, I think.

> Source/WebKit/blackberry/WebKitSupport/TouchEventHandler.cpp:78
> -    while (element->isInShadowTree())
> -        element = toElement(element->shadowAncestorNode());
> +    if (element->isInShadowTree())
> +        return true;

could you explain what does this patch fix, charles?

[Antonio Gomes]

Comment on attachment 138303 [details]
Patch

Ok, I think I understood the problem. Lets discuss on IRC a different approach

[Charles Wei]

Created attachment 138559 [details]
Patch

[Antonio Gomes]

Comment on attachment 138559 [details]
Patch

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

> Source/WebKit/blackberry/WebKitSupport/TouchEventHandler.cpp:79
> +static bool isRangeControlElement(Element* element)
> +{
> +    if (element->hasTagName(HTMLNames::inputTag)) {
> +        HTMLInputElement* inputElement = static_cast<HTMLInputElement*>(element);
> +        if (inputElement->isRangeControl())
> +            return true;
> +    }
> +    return false;
> +}

lets write this as

static bool isRangeControlElement(Element* elemenet) 
{
  assert(element);
  if (!element->hastagname(htmlnames::inputtag))
     return;

  htmlinputelement inputelement = static_cast<htmlinputelement*>(element);
  return inputelement->israngecontrol();
}

note the assert and the early return.

[Charles Wei]

Created attachment 138562 [details]
Patch

[Charles Wei]

Comment on attachment 138562 [details]
Patch

Addressed the comments from the reviewer and commit it.

[Antonio Gomes]

Comment on attachment 138562 [details]
Patch

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

> Source/WebKit/blackberry/WebKitSupport/TouchEventHandler.cpp:93
> -    // Make sure we are not operating a shadow node here, since the webpages
> -    // aren't able to attach event listeners to shadow content.
> +    // If the shadow tree contains a Range Control, like the MediaControlTimeline,
> +    // MediaContolVolumeElement, etc, they expect natual elements. Otherwise, we
> +    // won't operate on the shadow node, because the webpages aren't able to attach
> +    // listeners to shadow content.
>      ASSERT(element);
> -    while (element->isInShadowTree())
> +    while (element->isInShadowTree()) {
>          element = toElement(element->shadowAncestorNode());
> -
> +      
> +        if (isRangeControlElement(element))
> +            return true;
> +    }

maybe we should do a do-while here instead? so we test the original 'element'?

also, maybe we could move the block above to shouldConvertTouchToMouse enterily

[Antonio Gomes]

Comment on attachment 138562 [details]
Patch

ok, we can clean up as a follow up.

[WebKit Review Bot]

Comment on attachment 138562 [details]
Patch

Clearing flags on attachment: 138562

Committed r115065: <http://trac.webkit.org/changeset/115065>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Antonio Gomes]

reopening for cleaning things up further. see comment #8

[Charles Wei]

Created attachment 138774 [details]
Patch

[WebKit Review Bot]

Comment on attachment 138774 [details]
Patch

Clearing flags on attachment: 138774

Committed r115280: <http://trac.webkit.org/changeset/115280>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.