[Webkit-unassigned] [Bug 137597] Support activation behavior of link element

[bugzilla-daemon at webkit.org]

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





--- Comment #3 from Dhi Aurrahman <diorahman at rockybars.com>  2014-10-11 07:06:52 PST ---
(In reply to comment #2)
> (From update of attachment 239605 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=239605&action=review
> 
> The patch is great. The changelogs are great. The tests are really well done.
> 
> r- because of the duplicated isLinkClick() in HTMLLinkElement and HTMLAnchorElement. Otherwise this is nearly perfect.
> 
> > Source/WebCore/html/HTMLLinkElement.cpp:413
> > +bool HTMLLinkElement::isLinkClick(Event* event)
> > +{
> > +    return event->type() == eventNames().clickEvent && (!is<MouseEvent>(*event) || downcast<MouseEvent>(*event).button() != RightButton);
> > +}
> 
> Since this code is duplicated in HTMLAnchorElement, we should put it in a common place.
> 
> I suggest adding a static function "isRightClick" to the class MouseEvent. Then modify HTMLAnchorElement and HTMLLinkElement to use that function.

I'm not sure with adding a static function "isRightClick", since the current "isLinkClick" i.e.

return event->type() == eventNames().clickEvent && (!is<MouseEvent>(*event) || !downcast<MouseEvent>(*event).button() != RightButton);

`true` if:

1. It is a click event and not a MouseEvent (it doesn't care about the button, since it is not a MouseEvent)
2. It is a click event and a MouseEvent and the clicked button is not RightButton.

I can however create a bool MouseEvent::isRightClick() as follows,

bool MouseEvent::isRightClick()
{
    return type() == eventNames().clickEvent && button() == RightButton;
}

But it won't help to much on HTMLLinkElement::isLinkClick(Event *) and HTMLAnchorElement::isLinkClick(Event *)

bool isLinkClick(Event* event)
{
    return event->type() == eventNames().clickEvent && (!is<MouseEvent>(*event) || !downcast<MouseEvent>(*event).isRightClick());
}

Am I missing something here?

> 
> > Source/WebCore/html/HTMLLinkElement.cpp:420
> > +    if (url.isNull())
> > +        return;
> 
> I believe this is an interesting case, there should be a test for it if possible.
> 
> Maybe:
> 1) Create a click event with document.createEvent.
> 2) Send it to the <link> element with dispatchEvent()
> 3) Verify that the event has its default handled.
> 4) Set a time to 100ms. If the document has not navigated anywhere after the 100ms, finish the test with success.
> 
> This might even work when opened on Firefox...

I'm ready with the test (https://gist.github.com/diorahman/b1c2da24d44da2648c36), but I have to confirm the "isRightClick" first, before sending another patch

Thanks!

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.