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

Editorial: make note about "in body" insertion mode account for edge case #9780

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

not-my-profile
Copy link
Contributor

@not-my-profile not-my-profile commented Sep 23, 2023

This note found in the "Any other start tag" condition of the "in body" insertion mode, previously did not account for a "noscript" element being able to reach it, if the scripting flag is disabled (since the only preceding condition that can match a start tag whose tag name is "noscript" has the clause "if the scripting flag is enabled").

("noscript" is in the special category and thus non-ordinary.)


/parsing.html ( diff )

@not-my-profile not-my-profile changed the title Fix: Make note about "in body" insertion mode account for edge case Fix: make note about "in body" insertion mode account for edge case Sep 23, 2023
@keithamus
Copy link
Contributor

Can you please sign the participation agreement

@not-my-profile
Copy link
Contributor Author

Sure, done :)

@annevk
Copy link
Member

annevk commented Sep 25, 2023

Off-topic, but it seems a little weird that we'd only run

Reconstruct the active formatting elements, if any.

when scripting is disabled for noscript.

Copy link
Member

@hsivonen hsivonen left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

@hsivonen
Copy link
Member

Off-topic, but it seems a little weird that we'd only run

Reconstruct the active formatting elements, if any.

when scripting is disabled for noscript.

Why is that weird considering that we run it for both text and "any other element" in "in body"?

@hsivonen
Copy link
Member

Looks like my review wasn't enough for the rules that GitHub enforces.

@annevk
Copy link
Member

annevk commented Sep 26, 2023

I'm not sure, it seemed like something that could be done whether scripting is disabled or enabled. It isn't immediately clear to me that distinction is deliberate, but I also can't recall what the actual difference would be in parsing behavior…

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

I have some editorial nits, but thanks for verifying this is correct @hsivonen! And thanks for the fix @not-my-profile!

source Outdated
Comment on lines 125082 to 125084
<p class="note">This element will be an <span>ordinary</span> element
(with one exception: it can also be a <code>noscript</code> element,
if the <span>scripting flag</span> is disabled).</p>
Copy link
Member

Choose a reason for hiding this comment

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

I would make this something like:

This element will be an ordinary element. With one exception: when the scripting flag is disabled it can also be a noscript element.

and then wrap using the 100 column rule.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done :)

(Although I have kept if rather than when and also kept the comma in the new sentence.)

This note found in the "Any other start tag" condition of the "in body"
insertion mode, previously did not account for a "noscript" start tag
being able to reach it, if the scripting flag is disabled (since the
only preceding condition that can match a start tag whose tag name
is "noscript" has the clause "if the scripting flag is enabled").

("noscript" is in the special category and thus non-ordinary.)
@not-my-profile not-my-profile changed the title Fix: make note about "in body" insertion mode account for edge case Editorial: make note about "in body" insertion mode account for edge case Sep 26, 2023
@annevk annevk merged commit e4ce01e into whatwg:main Sep 26, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants