[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 #7 from Chris Dumez <cdumez at apple.com>  2014-10-11 17:31:57 PST ---
(From update of attachment 239683)
View in context: https://bugs.webkit.org/attachment.cgi?id=239683&action=review

>> Source/WebCore/dom/MouseEvent.cpp:193
>> +bool MouseEvent::triggerActivationBehavior(const Event* event)
> 
> Seeing this being used in practice, the name does not look right. The function name implies it triggers the Activation Behavior.
> 
> I think canTriggerActivationBehavior() would be better.

The argument should probably be a const Event& since it is expected to be non-null.

> Source/WebCore/html/HTMLLinkElement.cpp:410
> +void HTMLLinkElement::handleClick(Event* event)

Ditto. Argument could be an Event&

> Source/WebCore/html/HTMLLinkElement.cpp:420
> +    return;

Please omit this return.

> LayoutTests/fast/dom/html-link-element-activation-behavior-on-element-click-expected.txt:9
> +PASS event.data is 'test:ok'

This should appear before the TEST COMPLETE, something is likely wrong with the test?

> LayoutTests/fast/dom/html-link-element-activation-behavior-on-element-click.html:15
> +	shouldBe("event.data", "'test:ok'")

shouldBeEqualToString()

> LayoutTests/fast/dom/html-link-element-activation-behavior-on-element-click.html:16
> +	if (window.testRunner)

finishJSTest();

> LayoutTests/fast/dom/html-link-element-activation-behavior-on-element-click.html:22
> +if (window.testRunner) 

should be window.jsTestIsAsync = true;

> LayoutTests/fast/dom/html-link-element-activation-behavior-on-mouse-click-expected.txt:9
> +PASS event.data is 'test:ok'

The fact that the PASS is after the TEST COMPLETE is a bad sign. There is likely something wrong with the test and there is a fair chance it will be flaky.

> LayoutTests/fast/dom/html-link-element-activation-behavior-on-mouse-click.html:15
> +	shouldBe("event.data", "'test:ok'")

shouldBeEqualToString()

> LayoutTests/fast/dom/html-link-element-activation-behavior-on-mouse-click.html:16
> +	if (window.testRunner)

finishJSTest();

> LayoutTests/fast/dom/html-link-element-activation-behavior-on-mouse-click.html:22
> +if (window.testRunner) 

should be window.jsTestIsAsync = true;

> LayoutTests/fast/dom/html-link-element-activation-behavior-url-is-null.html:11
> +	if (window.testRunner)

should be a finishJSTest();

> LayoutTests/fast/dom/html-link-element-activation-behavior-url-is-null.html:16
> +shouldBe("target.dispatchEvent(mouseEvent)", "true");

shouldBeTrue()

> LayoutTests/fast/dom/html-link-element-activation-behavior-url-is-null.html:18
> +if (window.testRunner) 

should be window.jsTestIsAsync = true;

> LayoutTests/fast/dom/resources/html-link-element-activation-behavior-on-element-click-step1.html:15
> +		document.getElementById("target").click();

weird indentation

> LayoutTests/fast/dom/resources/html-link-element-activation-behavior-target.html:4
> +		window.parent.postMessage("test:ok", "*");

Weird indentation here. BTW, are you using tabs instead of spaces?

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