Commit 4eb4df63 authored by yu han's avatar yu han Committed by Commit Bot

Fixes click on inline element embedded in anchor inside <summary> breaks anchor.

Previous to this CL, clicking on an inline element embedded in an anchor placed
inside a <summary> tag will expand the <details> section instead of navigating to
the anchor's href. However, when the anchor is placed outside of <summary>,
it behaves correctly.

The error is caused by DOMActivate event generated by the inline element. As
DOMActivate bubbles up, it bypasses the anchor's event handler, reaches the
<summary>, and is handled there. Once DOMActivate is handled, the original
click event stops propagating and terminates. This behavior, however, differs
from when the anchor tag is placed outside of the summary. DOMActivate isn't
handled, and the original click event keeps bubbling up till it's handled by
the anchor.

DOMActivate event is deprecated:
https://developer.mozilla.org/en-US/docs/Web/API/Element/DOMActivate_event.
However, since blink still has code that depends on it, replacing it is outside
of the scope for this fix. Instead, this fix is for the anchor element to
handle the DOMActivate event as it bubbles up. The anchor event handler checks
the underlying event of DOMActivate and handles it if it's a click.

I also looked at an alternative fix by trying to prevent the DOMActivate event
from bubbling up. But calling event.stopPropagation() doesn't work as the
EventDispatcher::DispatchEventPostProcess doesn't check this status.


Bug: 538283
Change-Id: I11fb072faa0563279d43a28e5dc19cee89906bf0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1928234Reviewed-by: default avatarKent Tamura <tkent@chromium.org>
Reviewed-by: default avatarMason Freed <masonfreed@chromium.org>
Commit-Queue: Yu Han <yuzhehan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718552}
parent 60685f5e
...@@ -188,9 +188,10 @@ void HTMLAnchorElement::DefaultEventHandler(Event& event) { ...@@ -188,9 +188,10 @@ void HTMLAnchorElement::DefaultEventHandler(Event& event) {
DispatchSimulatedClick(&event); DispatchSimulatedClick(&event);
return; return;
} }
Event* click_event = GetClickEventOrNull(event);
if (IsLinkClick(event) && IsLiveLink()) { if (click_event && IsLiveLink()) {
HandleClick(event); HandleClick(*click_event);
event.SetDefaultHandled();
return; return;
} }
} }
...@@ -463,17 +464,24 @@ bool IsEnterKeyKeydownEvent(Event& event) { ...@@ -463,17 +464,24 @@ bool IsEnterKeyKeydownEvent(Event& event) {
!ToKeyboardEvent(event).repeat(); !ToKeyboardEvent(event).repeat();
} }
bool IsLinkClick(Event& event) { Event* GetClickEventOrNull(Event& event) {
if ((event.type() != event_type_names::kClick && // HTMLElement embedded in anchor tag dispatches a DOMActivate event when
event.type() != event_type_names::kAuxclick) || // clicked on. The original click event is set as underlying event.
!event.IsMouseEvent()) { bool use_underlying_event =
return false; event.type() == event_type_names::kDOMActivate && event.UnderlyingEvent();
Event* process_event =
use_underlying_event ? event.UnderlyingEvent() : &event;
if ((process_event->type() != event_type_names::kClick &&
process_event->type() != event_type_names::kAuxclick) ||
!process_event->IsMouseEvent()) {
return nullptr;
} }
auto& mouse_event = ToMouseEvent(event); auto& mouse_event = ToMouseEvent(*process_event);
int16_t button = mouse_event.button(); int16_t button = mouse_event.button();
return (button == static_cast<int16_t>(WebPointerProperties::Button::kLeft) || return (button == static_cast<int16_t>(WebPointerProperties::Button::kLeft) ||
button == button == static_cast<int16_t>(WebPointerProperties::Button::kMiddle))
static_cast<int16_t>(WebPointerProperties::Button::kMiddle)); ? process_event
: nullptr;
} }
bool HTMLAnchorElement::WillRespondToMouseClickEvents() { bool HTMLAnchorElement::WillRespondToMouseClickEvents() {
......
...@@ -132,7 +132,7 @@ inline LinkHash HTMLAnchorElement::VisitedLinkHash() const { ...@@ -132,7 +132,7 @@ inline LinkHash HTMLAnchorElement::VisitedLinkHash() const {
// Functions shared with the other anchor elements (i.e., SVG). // Functions shared with the other anchor elements (i.e., SVG).
bool IsEnterKeyKeydownEvent(Event&); bool IsEnterKeyKeydownEvent(Event&);
bool IsLinkClick(Event&); Event* GetClickEventOrNull(Event&);
} // namespace blink } // namespace blink
......
...@@ -112,7 +112,9 @@ void SVGAElement::DefaultEventHandler(Event& event) { ...@@ -112,7 +112,9 @@ void SVGAElement::DefaultEventHandler(Event& event) {
return; return;
} }
if (IsLinkClick(event)) { if (Event* click_event = GetClickEventOrNull(event)) {
click_event->SetDefaultHandled();
event.SetDefaultHandled();
String url = StripLeadingAndTrailingHTMLSpaces(HrefString()); String url = StripLeadingAndTrailingHTMLSpaces(HrefString());
if (url[0] == '#') { if (url[0] == '#') {
...@@ -121,7 +123,6 @@ void SVGAElement::DefaultEventHandler(Event& event) { ...@@ -121,7 +123,6 @@ void SVGAElement::DefaultEventHandler(Event& event) {
if (auto* svg_smil_element = if (auto* svg_smil_element =
DynamicTo<SVGSMILElement>(target_element)) { DynamicTo<SVGSMILElement>(target_element)) {
svg_smil_element->BeginByLinkActivation(); svg_smil_element->BeginByLinkActivation();
event.SetDefaultHandled();
return; return;
} }
} }
...@@ -129,16 +130,14 @@ void SVGAElement::DefaultEventHandler(Event& event) { ...@@ -129,16 +130,14 @@ void SVGAElement::DefaultEventHandler(Event& event) {
AtomicString target(svg_target_->CurrentValue()->Value()); AtomicString target(svg_target_->CurrentValue()->Value());
if (target.IsEmpty() && FastGetAttribute(xlink_names::kShowAttr) == "new") if (target.IsEmpty() && FastGetAttribute(xlink_names::kShowAttr) == "new")
target = AtomicString("_blank"); target = AtomicString("_blank");
event.SetDefaultHandled();
if (!GetDocument().GetFrame()) if (!GetDocument().GetFrame())
return; return;
FrameLoadRequest frame_request( FrameLoadRequest frame_request(
&GetDocument(), ResourceRequest(GetDocument().CompleteURL(url))); &GetDocument(), ResourceRequest(GetDocument().CompleteURL(url)));
frame_request.SetNavigationPolicy(NavigationPolicyFromEvent(&event)); frame_request.SetNavigationPolicy(NavigationPolicyFromEvent(click_event));
frame_request.SetTriggeringEventInfo( frame_request.SetTriggeringEventInfo(
event.isTrusted() ? TriggeringEventInfo::kFromTrustedEvent click_event->isTrusted() ? TriggeringEventInfo::kFromTrustedEvent
: TriggeringEventInfo::kFromUntrustedEvent); : TriggeringEventInfo::kFromUntrustedEvent);
frame_request.GetResourceRequest().SetHasUserGesture( frame_request.GetResourceRequest().SetHasUserGesture(
LocalFrame::HasTransientUserActivation(GetDocument().GetFrame())); LocalFrame::HasTransientUserActivation(GetDocument().GetFrame()));
......
<!DOCTYPE html>
<meta charset="utf-8">
<title>summary element: clicking on anchor containing inline element</title>
<link rel="author" title="Yu Han" href="mailto:yuzhehan@chromium.org">
<link rel="help" href="https://html.spec.whatwg.org/C/#the-summary-element">
<link rel="help" href="https://html.spec.whatwg.org/multipage/text-level-semantics.html#the-a-element">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<details id="details_i">
<summary>Anchor text is wrapped with &lt;i&gt; tag <a href="#with_i_tag"><i id="with_i">permalink</i></a></summary>
<p>asdf</p>
</details>
<details id="details_span">
<summary>This one uses &lt;span&gt;. <a href="#with_span_tag"><span id="with_span">permalink</span></a></summary>
<p>asdf</p>
</details>
<details id="details_svg">
<summary>
<svg style="width: 100px;" viewBox="0 0 100 100" xmlns="http://www.w3.org/2000/svg">
<a href="#inside_svg_w_circle">
<circle id="svg_circle" cx="50" cy="40" r="35"/>
</a>
<a href="#inside_svg_w_text">
<text id="svg_text" x="50" y="90" text-anchor="middle">
&lt;circle&gt;
</text>
</a>
</svg>
</summary>
<p>asdf</p>
</details>
<script>
function testClickingOnInlineElement(detailsId, targetId, expected, testName) {
const details = document.getElementById(detailsId);
const target = document.getElementById(targetId);
const test = async_test(testName);
const promise = new Promise((resolve, reject) => {
window.onhashchange = test.step_func_done(() => {
assert_false(details.open);
assert_true(location.hash === expected);
resolve();
});
});
if (target.click) {
target.click();
}
else {
// svg element don't have click method
target.dispatchEvent(new MouseEvent('click', {
view: window,
bubbles: true,
cancelable: true
}));
}
return promise;
};
async function testAll() {
try {
await testClickingOnInlineElement("details_i", "with_i", "#with_i_tag", "Expected <a> containing <i> to navigate");
await testClickingOnInlineElement("details_span", "with_span", "#with_span_tag", "Expected <a> containing <span> to navigate");
await testClickingOnInlineElement("details_svg", "svg_circle", "#inside_svg_w_circle", "Expected <a>, inside svg, containing <circle> to navigate");
await testClickingOnInlineElement("details_svg", "svg_text", "#inside_svg_w_text", "Expected <a>, inside svg, containing <text> to navigate");
} catch (exception) {
assert_unreached("should NOT-THROW exception");
}
};
var allTests = async_test("Clicking on anchor with embedded inline element should navigate instead of opening details");
testAll().then(()=>{ allTests.done(); });
</script>
\ No newline at end of file
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment