Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTML parser: make breaking out of foreign content apply in innerHTML #6399

Merged
merged 1 commit into from
Mar 2, 2021

Conversation

zcorpan
Copy link
Member

@zcorpan zcorpan commented Feb 19, 2021

This is intended to match Chromium and WebKit.

Fixes #5117.

(See WHATWG Working Mode: Changes for more details.)


/parsing.html ( diff )

This is intended to match Chromium and WebKit.

Fixes #5117.
@mozfreddyb
Copy link
Contributor

Thanks @zcorpan, here's an old Gecko implementation bug, which ought to be repurposed https://bugzilla.mozilla.org/show_bug.cgi?id=1205631

@zcorpan
Copy link
Member Author

zcorpan commented Feb 22, 2021

Great, thanks @mozfreddyb. I plan to write tests for this on Friday this week, unless someone else beats me to it. :)

@domenic
Copy link
Member

domenic commented Feb 22, 2021

/cc @whatwg/html-parser; it'd be ideal to get review from one of those folks too, although this seems simple enough :)

@domenic domenic added interop Implementations are not interoperable with each other normative change topic: parser labels Feb 22, 2021
@hsivonen
Copy link
Member

LGTM. Thanks.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Editorial LGTM. Once tests are written we can double check that this matches Chromium and WebKit, and if so, we can check the implementer interest box and merge.

zcorpan added a commit to zcorpan/html5lib-tests that referenced this pull request Feb 26, 2021
zcorpan added a commit to web-platform-tests/wpt that referenced this pull request Feb 26, 2021
See html5lib/html5lib-tests#133

Spec change: whatwg/html#6399

To regenerate the tests with Python3 requires this change: #27798
@zcorpan
Copy link
Member Author

zcorpan commented Feb 26, 2021

@domenic
Copy link
Member

domenic commented Feb 26, 2021

Tests indeed match 2/3 browsers, so please merge at will!

I hope someone more familiar with html5lib and the surrounding infrastructure can review the test PRs...

@annevk
Copy link
Member

annevk commented Feb 27, 2021

I'm happy with the tests, but I'm no longer an html5lib peer it seems. Paging @gsnedders and @jgraham.

@domenic domenic merged commit f690ad9 into main Mar 2, 2021
@domenic domenic deleted the bocoup/fragment-parsing-svg-p branch March 2, 2021 17:08
zcorpan added a commit that referenced this pull request Mar 5, 2021
The regression was introduced in f690ad9
(PR #6399)

For this case:
```
<table><math><p>foo
```
the `<p>` token would not have foster parenting enabled, thus inserting it
into the table.

Fixes #6439.
zcorpan added a commit to web-platform-tests/wpt that referenced this pull request Mar 29, 2021
TRowbotham added a commit to TRowbotham/PHPDOM that referenced this pull request Apr 1, 2021
Fixes html tags being nested inside svg and math tags. Uses the proposed
spec changes from whatwg/html#6399 and whatwg/html#6455
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 2, 2021
…ith html5lib-tests#133, a=testonly

Automatic update from web-platform-tests
HTML: Test <svg><p> in innerHTML: Sync with html5lib-tests#133

See html5lib/html5lib-tests#133

Spec change: whatwg/html#6399
--

wpt-commits: 44e250d007a5c795e9005b34ac33ee966195190b
wpt-pr: 27799
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 2, 2021
…ith html5lib-tests#133, a=testonly

Automatic update from web-platform-tests
HTML: Test <svg><p> in innerHTML: Sync with html5lib-tests#133

See html5lib/html5lib-tests#133

Spec change: whatwg/html#6399
--

wpt-commits: 44e250d007a5c795e9005b34ac33ee966195190b
wpt-pr: 27799
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 7, 2021
…ith html5lib-tests#133, a=testonly

Automatic update from web-platform-tests
HTML: Test <svg><p> in innerHTML: Sync with html5lib-tests#133

See html5lib/html5lib-tests#133

Spec change: whatwg/html#6399
--

wpt-commits: 44e250d007a5c795e9005b34ac33ee966195190b
wpt-pr: 27799
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 8, 2021
…ith html5lib-tests#133, a=testonly

Automatic update from web-platform-tests
HTML: Test <svg><p> in innerHTML: Sync with html5lib-tests#133

See html5lib/html5lib-tests#133

Spec change: whatwg/html#6399
--

wpt-commits: 44e250d007a5c795e9005b34ac33ee966195190b
wpt-pr: 27799
domenic pushed a commit that referenced this pull request Jul 20, 2021
The regression was introduced in f690ad9 (#6399).

For the case:

<table><math><p>foo

the <p> token would not have foster parenting enabled, thus inserting it into the table.

Fixes #6439.
stevecheckoway added a commit to sparklemotion/nokogiri that referenced this pull request Oct 13, 2021
The first comment is incorrect. The fragment case was changed to match
the nonfragment case in whatwg/html#6399.

This introduced a bug that was noted by the second comment indicating
that the parser diverged from the standard to fix the bug. The standard
was subsequently fixed in whatwg/html#6455,
obviating the comment.
mfreed7 pushed a commit to mfreed7/html that referenced this pull request Jun 3, 2022
The regression was introduced in f690ad9 (whatwg#6399).

For the case:

<table><math><p>foo

the <p> token would not have foster parenting enabled, thus inserting it into the table.

Fixes whatwg#6439.
mgol added a commit to mgol/jquery that referenced this pull request Oct 20, 2024
…Parser`

Firefox has a bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=1205631
where assignment to `innerHTML` produces a different DOM tree than passing the
HTML to `document.write()`; other browsers use the same algorithm in both
places, the same one Firefox uses for `document.write()`. Firefox's behavior
used to be spec-compliant but the spec changed in:
whatwg/html#6399

Because `$.parseHTML` used to use:
```js
document.implementation.createHTMLDocument( "" )
```
followed by an `innerHTML` assignment under the hood, Firefox was hitting that
issue.

TODO we cannot pass `data` to `parseFromString` like that as it goes through
`buildFragment` later anyway.

Ref whatwg/html#5117
Ref whatwg/html#6399
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interop Implementations are not interoperable with each other normative change topic: parser
Development

Successfully merging this pull request may close these issues.

Parsing of <svg><p> in innerHTML is not interoperable
5 participants