Skip to content

GitLab

T
tangled
Commit b4cb14f3 authored by Yu Han's avatar Yu Han Committed by Commit Bot
Browse files
Options

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

This reverts commit 4eb4df63.

Reason for revert: Caused crbug.com/1045433.

Original change's description:
> 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/+/1928234
> Reviewed-by: Kent Tamura <tkent@chromium.org>
> Reviewed-by: Mason Freed <masonfreed@chromium.org>
> Commit-Queue: Yu Han <yuzhehan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#718552}

TBR=tkent@chromium.org,masonfreed@chromium.org,yuzhehan@chromium.org

Bug: 1045433
Change-Id: I14b369beb04171ef846cb3a79ebb3fe268cf5c89
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2023267Reviewed-by: default avatarMason Freed <masonfreed@chromium.org>
Commit-Queue: Yu Han <yuzhehan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735575}
parent 99965b74
Hide whitespace changes
Inline Side-by-side
Showing with 19 additions and 105 deletions
third_party/blink/renderer/core/html/html_anchor_element.cc
View file @ b4cb14f3
......@@ -180,10 +180,9 @@ void HTMLAnchorElement::DefaultEventHandler(Event& event) {
DispatchSimulatedClick(&event);
return;
}
Event* click_event = GetClickEventOrNull(event);
if (click_event && IsLiveLink()) {
HandleClick(*click_event);
event.SetDefaultHandled();
if (IsLinkClick(event) && IsLiveLink()) {
HandleClick(event);
return;
}
}
......@@ -465,26 +464,17 @@ bool IsEnterKeyKeydownEvent(Event& event) {
!ToKeyboardEvent(event).repeat();
}
Event* GetClickEventOrNull(Event& event) {
// HTMLElement embedded in anchor tag dispatches a DOMActivate event when
// clicked on. The original click event is set as underlying event.
bool use_underlying_event = event.type() == event_type_names::kDOMActivate &&
event.UnderlyingEvent() &&
!event.UnderlyingEvent()->DefaultHandled() &&
event.UnderlyingEvent()->target();
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;
bool IsLinkClick(Event& event) {
if ((event.type() != event_type_names::kClick &&
event.type() != event_type_names::kAuxclick) ||
!event.IsMouseEvent()) {
return false;
}
auto& mouse_event = ToMouseEvent(*process_event);
auto& mouse_event = ToMouseEvent(event);
int16_t button = mouse_event.button();
return (button == static_cast<int16_t>(WebPointerProperties::Button::kLeft) ||
button == static_cast<int16_t>(WebPointerProperties::Button::kMiddle))
? process_event
: nullptr;
button ==
static_cast<int16_t>(WebPointerProperties::Button::kMiddle));
}
bool HTMLAnchorElement::WillRespondToMouseClickEvents() {
......
third_party/blink/renderer/core/html/html_anchor_element.h
View file @ b4cb14f3
......@@ -132,7 +132,7 @@ inline LinkHash HTMLAnchorElement::VisitedLinkHash() const {
// Functions shared with the other anchor elements (i.e., SVG).
bool IsEnterKeyKeydownEvent(Event&);
Event* GetClickEventOrNull(Event&);
bool IsLinkClick(Event&);
} // namespace blink
......
third_party/blink/renderer/core/svg/svg_a_element.cc
View file @ b4cb14f3
......@@ -113,9 +113,7 @@ void SVGAElement::DefaultEventHandler(Event& event) {
return;
}
if (Event* click_event = GetClickEventOrNull(event)) {
click_event->SetDefaultHandled();
event.SetDefaultHandled();
if (IsLinkClick(event)) {
String url = StripLeadingAndTrailingHTMLSpaces(HrefString());
if (url[0] == '#') {
......@@ -124,6 +122,7 @@ void SVGAElement::DefaultEventHandler(Event& event) {
if (auto* svg_smil_element =
DynamicTo<SVGSMILElement>(target_element)) {
svg_smil_element->BeginByLinkActivation();
event.SetDefaultHandled();
return;
}
}
......@@ -131,15 +130,17 @@ void SVGAElement::DefaultEventHandler(Event& event) {
AtomicString target(svg_target_->CurrentValue()->Value());
if (target.IsEmpty() && FastGetAttribute(xlink_names::kShowAttr) == "new")
target = AtomicString("_blank");
event.SetDefaultHandled();
if (!GetDocument().GetFrame())
return;
FrameLoadRequest frame_request(
&GetDocument(), ResourceRequest(GetDocument().CompleteURL(url)));
frame_request.SetNavigationPolicy(NavigationPolicyFromEvent(click_event));
frame_request.SetNavigationPolicy(NavigationPolicyFromEvent(&event));
frame_request.SetTriggeringEventInfo(
click_event->isTrusted() ? TriggeringEventInfo::kFromTrustedEvent
: TriggeringEventInfo::kFromUntrustedEvent);
event.isTrusted() ? TriggeringEventInfo::kFromTrustedEvent
: TriggeringEventInfo::kFromUntrustedEvent);
frame_request.GetResourceRequest().SetHasUserGesture(
LocalFrame::HasTransientUserActivation(GetDocument().GetFrame()));
......
third_party/blink/web_tests/external/wpt/html/semantics/interactive-elements/the-summary-element/anchor-with-inline-element.html deleted 100644 → 0
View file @ 99965b74
<!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