Skip to content

Commit 6752480

Browse files
sideshowbarkerAhmad Saleem
authored andcommitted
Change navigable target names to _blank if they have dangling markup
https://bugs.webkit.org/show_bug.cgi?id=257349 Reviewed by Chris Dumez. whatwg/html#9309 * LayoutTests/imported/w3c/resources/import-expectations.json: * LayoutTests/imported/w3c/web-platform-tests/html/browsers/windows/dangling-markup-window-name.tentative-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/html/browsers/windows/dangling-markup-window-name.tentative.html: Added. * LayoutTests/imported/w3c/web-platform-tests/html/browsers/windows/resources/window-name.sub.html: * LayoutTests/imported/w3c/web-platform-tests/html/browsers/windows/w3c-import.log: * LayoutTests/tests-options.json: * Source/WebCore/dom/Element.cpp: (WebCore::Element::makeTargetBlankIfHasDanglingMarkup const): * Source/WebCore/dom/Element.h: * Source/WebCore/html/HTMLAnchorElement.cpp: (WebCore::HTMLAnchorElement::effectiveTarget const): * Source/WebCore/html/HTMLFormElement.cpp: (WebCore::HTMLFormElement::effectiveTarget const): Canonical link: https://commits.webkit.org/267154@main
1 parent b2fab9c commit 6752480

10 files changed

Lines changed: 123 additions & 4 deletions

File tree

LayoutTests/imported/w3c/resources/import-expectations.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,7 @@
229229
"web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects.sub.html": "skip",
230230
"web-platform-tests/html/browsers/sandboxing": "skip",
231231
"web-platform-tests/html/browsers/the-window-object": "import",
232+
"web-platform-tests/html/browsers/windows": "import",
232233
"web-platform-tests/html/browsers/windows/auxiliary-browsing-contexts": "import",
233234
"web-platform-tests/html/browsers/windows/browsing-context-first-created.xhtml": "skip",
234235
"web-platform-tests/html/browsers/windows/browsing-context-names": "import",
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
2+
PASS Dangling Markup in target is not reset when set by window.open
3+
PASS Dangling Markup with "\n" in target is reset when set by <a> tag
4+
PASS Dangling Markup with "\r" in target is reset when set by <a> tag
5+
PASS Dangling Markup with "\t" in target is reset when set by <a> tag
6+
PASS Dangling Markup in target is reset when set by <form> tag
7+
PASS Dangling Markup in target is reset when set by <base> tag
8+
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
<!doctype html>
2+
<html>
3+
<head>
4+
<title>Dangling Markup in target</title>
5+
<meta name="timeout" content="long">
6+
<script src="/resources/testharness.js"></script>
7+
<script src="/resources/testharnessreport.js"></script>
8+
<script src="/common/utils.js"></script>
9+
</head>
10+
<body>
11+
<script>
12+
function anchorClick(target, id) {
13+
const hyperlink = document.body.appendChild(document.createElement('a'));
14+
if (target) {
15+
hyperlink.target = target;
16+
}
17+
hyperlink.href = `resources/window-name.sub.html?report=${id}|close`;
18+
hyperlink.click();
19+
}
20+
21+
async function pollResultAndCheck(t, id, expected) {
22+
const stashURL = new URL('resources/window-name-stash.py', location);
23+
stashURL.searchParams.set('id', id);
24+
25+
let res = 'NONE';
26+
while (res == 'NONE') {
27+
await new Promise(resolve => { t.step_timeout(resolve, 100); });
28+
29+
const response = await fetch(stashURL);
30+
res = await response.text();
31+
}
32+
if (res !== expected) {
33+
assert_unreached('Stash result does not equal expected result.')
34+
}
35+
}
36+
37+
promise_test(async t => {
38+
const id = token();
39+
const value = '\n<' + id;
40+
41+
window.open(`resources/window-name.sub.html?report=${id}|close`, value);
42+
await pollResultAndCheck(t, id, value);
43+
}, 'Dangling Markup in target is not reset when set by window.open');
44+
45+
promise_test(async t => {
46+
const id = token();
47+
const value = '\n<' + id;
48+
49+
anchorClick(value, id)
50+
await pollResultAndCheck(t, id, '');
51+
}, 'Dangling Markup with "\\n" in target is reset when set by <a> tag');
52+
53+
promise_test(async t => {
54+
const id = token();
55+
const value = '\r<' + id;
56+
57+
anchorClick(value, id)
58+
await pollResultAndCheck(t, id, '');
59+
}, 'Dangling Markup with "\\r" in target is reset when set by <a> tag');
60+
61+
promise_test(async t => {
62+
const id = token();
63+
const value = '\t<' + id;
64+
65+
anchorClick(value, id)
66+
await pollResultAndCheck(t, id, '');
67+
}, 'Dangling Markup with "\\t" in target is reset when set by <a> tag');
68+
69+
promise_test(async t => {
70+
const id = token();
71+
const value = '\n<' + id;
72+
73+
const form = document.body.appendChild(document.createElement('form'));
74+
form.target = value;
75+
form.method = 'GET';
76+
form.action = 'resources/window-name.sub.html';
77+
const input = form.appendChild(document.createElement('input'));
78+
input.type = 'hidden';
79+
input.name = 'report';
80+
input.value = `${id}|close`;
81+
form.submit();
82+
83+
await pollResultAndCheck(t, id, '');
84+
}, 'Dangling Markup in target is reset when set by <form> tag');
85+
86+
promise_test(async t => {
87+
const id = token();
88+
const value = '\n<' + id;
89+
const base = document.head.appendChild(document.createElement('base'));
90+
base.target = value;
91+
92+
anchorClick(null, id)
93+
await pollResultAndCheck(t, id, '');
94+
}, 'Dangling Markup in target is reset when set by <base> tag');
95+
</script>
96+
</body>
97+
</html>

LayoutTests/imported/w3c/web-platform-tests/html/browsers/windows/resources/window-name.sub.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
<title>popup helper</title>
33
<script>
44

5-
const search = window.location.search.replace("?", "");
5+
const search = decodeURIComponent(window.location.search.replace("?", ""));
66
const steps = search.split("|");
77

88
async function proceedTest() {

LayoutTests/imported/w3c/web-platform-tests/html/browsers/windows/w3c-import.log

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ List of files:
1717
/LayoutTests/imported/w3c/web-platform-tests/html/browsers/windows/browsing-context-window.html
1818
/LayoutTests/imported/w3c/web-platform-tests/html/browsers/windows/browsing-context.html
1919
/LayoutTests/imported/w3c/web-platform-tests/html/browsers/windows/clear-window-name.https.html
20+
/LayoutTests/imported/w3c/web-platform-tests/html/browsers/windows/dangling-markup-window-name.tentative.html
2021
/LayoutTests/imported/w3c/web-platform-tests/html/browsers/windows/document-domain-nested-navigate.window.js
2122
/LayoutTests/imported/w3c/web-platform-tests/html/browsers/windows/document-domain-nested-set.window.js
2223
/LayoutTests/imported/w3c/web-platform-tests/html/browsers/windows/document-domain-nested.window.js

LayoutTests/tests-options.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3383,6 +3383,9 @@
33833383
"imported/w3c/web-platform-tests/html/browsers/windows/clear-window-name.https.html": [
33843384
"slow"
33853385
],
3386+
"imported/w3c/web-platform-tests/html/browsers/windows/dangling-markup-window-name.tentative.html": [
3387+
"slow"
3388+
],
33863389
"imported/w3c/web-platform-tests/html/browsers/windows/noreferrer-window-name.html": [
33873390
"slow"
33883391
],

Source/WebCore/dom/Element.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5399,4 +5399,11 @@ void Element::contentVisibilityViewportChange(bool)
53995399
document().scheduleContentRelevancyUpdate(ContentRelevancy::OnScreen);
54005400
}
54015401

5402+
AtomString Element::makeTargetBlankIfHasDanglingMarkup(const AtomString& target)
5403+
{
5404+
if ((target.contains('\n') || target.contains('\r') || target.contains('\t')) && target.contains('<'))
5405+
return "_blank"_s;
5406+
return target;
5407+
}
5408+
54025409
} // namespace WebCore

Source/WebCore/dom/Element.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -768,6 +768,8 @@ class Element : public ContainerNode {
768768

769769
void updateLabel(TreeScope&, const AtomString& oldForAttributeValue, const AtomString& newForAttributeValue);
770770

771+
static AtomString makeTargetBlankIfHasDanglingMarkup(const AtomString& target);
772+
771773
private:
772774
LocalFrame* documentFrameWithNonNullView() const;
773775
void hideNonceSlow();

Source/WebCore/html/HTMLAnchorElement.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -649,7 +649,7 @@ AtomString HTMLAnchorElement::effectiveTarget() const
649649
auto effectiveTarget = target();
650650
if (effectiveTarget.isEmpty())
651651
effectiveTarget = document().baseTarget();
652-
return effectiveTarget;
652+
return makeTargetBlankIfHasDanglingMarkup(effectiveTarget);
653653
}
654654

655655
HTMLAnchorElement::EventType HTMLAnchorElement::eventType(Event& event)

Source/WebCore/html/HTMLFormElement.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -735,12 +735,12 @@ AtomString HTMLFormElement::effectiveTarget(const Event* event, HTMLFormControlE
735735
if (RefPtr submitter = overrideSubmitter ? overrideSubmitter : findSubmitter(event)) {
736736
auto& targetValue = submitter->attributeWithoutSynchronization(formtargetAttr);
737737
if (!targetValue.isNull())
738-
return targetValue;
738+
return makeTargetBlankIfHasDanglingMarkup(targetValue);
739739
}
740740

741741
auto targetValue = target();
742742
if (!targetValue.isNull())
743-
return targetValue;
743+
return makeTargetBlankIfHasDanglingMarkup(targetValue);
744744

745745
return document().baseTarget();
746746
}

0 commit comments

Comments
 (0)