The existing implementation has some pre-existing issues where it is
incorrectly assumes that byte offsets are given through the IDL instead
of UTF-16 code units. While making these changes, leave some FIXMEs for
that.
We currently track the [line, column] position of every HTMLToken, as
this is what is needed for LibGUI's syntax highlighting. Some non-LibGUI
purposes (e.g. highlighting HTML with HTML) require a byte offset. Track
both during tokenization.
The FIXME here describes an old constraint on JS Interpreters which no
longer holds. It hails from a time when we had the global object and
JS realm attached to the document.
This is intended to annotate conversions from unknown floating-point
values to CSSPixels, and make it more obvious the fp value will be
rounded to the nearest fixed-point value.
In general it is not safe to convert any arbitrary floating-point value
to CSSPixels. CSSPixels has a resolution of 0.015625, which for small
values (e.g. scale factors between 0 and 1), can produce bad results
if converted to CSSPixels then scaled back up. In the worst case values
can underflow to zero and produce incorrect results.
We were previously setting the end position of attribute names in self-
closing HTML tags to the end of the attribute value. To illustrate the
previous behavior, consider this tag and its attribute's start and end
positions (shown inclusively below):
<meta charset="UTF-8" />
^ name start
^ value start
^ value end
^ name end
Rather than setting the end position of the attribute name when we parse
the closing slash, ensure the end position is already set while we are
in the AttributeName state. We now have:
<meta charset="UTF-8" />
^ name start
^ name end
^ value start
^ value end
The tokenizer unit test has been extended to test these positions.
To illustrate the previous behavior, consider these tags and their start
and end positions (shown inclusively below):
Start tag: End tag:
<span> </span>
^ start ^ start
^end ^end
The start position of a tag is the first ASCII-alpha code point after
the opening brace. The start position of a close tag is the slash just
before the first ASCII-alpha code point. And the end position of both
is the closing brace. So the opening brace is not included in the
emitted tag, but the closing brace is. And the end tag including the
slash is an oddity that had to be worked around in its only use case
(syntax highlighting).
We now consistently exclude the braces from the emitted tag, and also
exclude the slash from the end tag, so that it does not need to be
accounted for in syntax highlighting. That is, we now have:
Start tag: End tag:
<span> </span>
^ start ^ start
^end ^end
The tokenizer unit test has been extended to test these positions.
If we run an inline script from the HTML parser, it may append a text
node to the current insertion point.
If there was text content immediately following the script element,
we would previously overwrite the script-inserted text content, due to
an oversight in the way we select an appropriate insertion point
This patch fixes the issue by only inserting parser content into
existing text nodes if they are empty.
Stop worrying about tiny OOMs. Work towards #20449.
While going through these, I also changed the function signature in many
places where returning ThrowCompletionOr<T> is no longer necessary.
We now apply MathML's default user agent style sheet along with other
default styles. This sheet is not mixed in with the other styles in
CSS/Default.css because it is a namespaced stylesheet and so has to
be its own sheet.
Some websites (like Reddit) like to instantiate "components" by setting
innerHTML to a huge chunk of stuff. Sometimes those huge chunks of stuff
contain inline style sheets (i.e `<style>` elements).
Before this change, we would end up parsing the CSS in those elements
multiple times, because we had no way of knowing that we were within
a fragment parser's temporary document.
This patch avoids the extra CSS parsing work by adding adding a flag to
Document that tells us it's being used by the fragment parser. Then, we
simply avoid parsing CSS for style elements in such documents. The CSS
then gets parsed immediately upon insertion into the proper DOM.
There were multiple bugs in the parsing algorithm for handling text
occurring inside a `table` element:
- When there was pending non-whitespace text inside a table, we only
flushed one token instead of all pending tokens.
- Also, we didn't even flush one of the right tokens, but instead the
token that caused the flush to happen.
- Once we started flushing the right tokens, it turned out we had not
yet implemented character insertion points expressed as "before X".
- Finally, we were not exiting the "in table text" mode after flushing
pending tokens, effectively getting us stuck in that mode until EOF.
To abort the processing of any nested invocations of the tokenizer,
just return is enough in this case.
During the process of pending parsing blocking script, the
is_ready_to_be_parser_executed() check should be applied on the
blocking script, not the original script.
This will examine the algorithm known as "the end" from the HTML
specification, which executes when parsing HTML markup has completed,
and it's potential to observably run script or change certain
attributes.
This currently executes in our engine when parsing HTML received from
the internet during navigation, using document.{open,write,close},
setting the innerHTML attribute or using DOMParser. The latter two are
only possible by executing script.
This has been causing some issues in our engine, which will be shown
later, so we are considering removing the call to "the end" for these
two cases.
Spoiler: the implications of running "the end" for DOMParser will be
considered in the future. It is the only script-created HTML/XML parser
remaining after this commit that uses "the end", including it's XML
variant implemented as XMLDocumentBuilder::document_end().
This will only focus on setting the innerHTML attribute, which falls
under "HTML fragment parsing", which starts here in the specification:
https://html.spec.whatwg.org/multipage/parsing.html#parsing-html-fragments44dd824764/Userland/Libraries/LibWeb/HTML/Parser/HTMLParser.cpp (L3491)
While you may notice our HTMLParser::parse_html_fragment returns `void`
and assume this means no scripts are executed because of our use of
`WebIDL::ExceptionOr<T>` and `JS::ThrowCompletionOr<T>`, note that
dispatched events will execute arbitrary script via a callback, catch
any exceptions, report them and not propagate them. This means that
while a function does not return an exception type, it can still
potentially execute script.
A breakdown of the steps of "the end" in the context of HTML fragment
parsing and its observability follows:
https://html.spec.whatwg.org/multipage/parsing.html#the-end44dd824764/Userland/Libraries/LibWeb/HTML/Parser/HTMLParser.cpp (L221)
1. No-op, as we don't currently have speculative HTML parsing. Even if
we did, we would instantly return after stopping the speculative
HTML parser anyway.
2. No-op, document.{open,write,close} are not accessible from the
temporary document.
3. No-op, document.readyState, window.navigation.timing and the
readystatechange event are not accessible from the created temporary
document.
4. This is presumably done so that reentrant invocation of the HTML
parser from document.{write,close} during the firing of the events
after step 4 ends up parsing from a clean state. This is a no-op, as
the events after step 4 do not fire and are not accessible.
5. No-op, we set HTMLScriptElement::m_already_started to true when
creating it whilst parsing an HTML fragment, which causes
HTMLScriptElement::prepare_script to instantly bail, meaning
`scripts_to_execute_when_parsing_has_finished` is always empty.
6. No-op, tasks are considered not runnable when the document does not
have a browsing context, which is always the case in fragment
parsing. Additionally, window.navigation.timing and the
DOMContentLoaded event aren't reachable from the temporary document.
7. Almost a no-op, `scripts_to_execute_as_soon_as_possible` is always
empty for the same reason as step 4. However, this step uses an
unconditional `spin_until` call, which _is_ observable and causes
one of the alluded to issues, which will be talked about later.
8. No-op, as delaying the load event has no purpose in this case, as
the task in step 9 will set the current document readiness to
"complete" and then return immediately after, as the temporary
document has no browsing context, skipping the Window load event.
However, this step causes another alluded to issue, which will be
talked about later.
9. No-op, for the same reason as step 6. Additionally,
document.readyState is not accessible from the temporary document
and the temporary document has no browsing context, so navigation
timing, the Window load event, the pageshow event, the Document load
event and the `<iframe>` load steps are not executed at all.
10. No-op, as this flag is only set from window.print(), which is not
accessible for this document.
11. No-op, as the temporary document is not accessible from anything
else and will be immediately destroyed after HTML fragment parsing.
Additionally, browsing context containers (`<iframe>`, `<frame>` and
`<object>`) cannot run in documents with no browsing context:
- `<iframe>` and `<frame>` use "create a new child navigable":
https://html.spec.whatwg.org/multipage/document-sequences.html#create-a-new-child-navigable44dd824764/Userland/Libraries/LibWeb/HTML/BrowsingContextContainer.cpp (L43-L45)
> 2. Let group be element's node document's browsing context's
top-level browsing context's group.
This requires the element's node document's browsing context to be
non-null, but it is always null with the temporary document created for
HTML fragment parsing.
This is protected against here for `<iframe>`:
https://html.spec.whatwg.org/multipage/iframe-embed-object.html#the-iframe-element:the-iframe-element-644dd824764/Userland/Libraries/LibWeb/HTML/HTMLIFrameElement.cpp (L45)
> When an iframe element element is inserted into a document whose
browsing context is non-null, the user agent must run these steps:
1. Create a new child navigable for element.
This is currently not protected against for `<frame>` in the
specification:
https://html.spec.whatwg.org/multipage/obsolete.html#active-frame-element
> A frame element is said to be an active frame element when it is in a
document.
> When a frame element element is created as an active frame element,
or becomes an active frame element after not having been one, the
user agent must run these steps:
> 1. Create a new child navigable for element.
However, since this would cause a null dereference, this is actually a
specification issue. See: https://github.com/whatwg/html/issues/9136
- `<object>` uses "queue an element task" and has a browsing context
null check.
https://html.spec.whatwg.org/multipage/iframe-embed-object.html#the-object-element:queue-an-element-task44dd824764/Userland/Libraries/LibWeb/HTML/HTMLObjectElement.cpp (L58)44dd824764/Userland/Libraries/LibWeb/HTML/HTMLObjectElement.cpp (L105)
> ...the user agent must queue an element task on the DOM manipulation
task source given the object element to run the following steps to
(re)determine what the object element represents.
As established above, tasks are not runnable in documents with null
browsing contexts. However, for avoidance of doubt, it checks if the
document's browsing context is null, and if so, it falls back to
representing the element's children and gets rid of any child navigable
the `<object>` element may have.
> 2. If the element has an ancestor media element, or has an ancestor
object element that is not showing its fallback content, or if the
element is not in a document whose browsing context is non-null,
or if the element's node document is not fully active, or if the
element is still in the stack of open elements of an HTML parser
or XML parser, or if the element is not being rendered, then jump
to the step below labeled fallback.
> 4. Fallback: The object element represents the element's children.
This is the element's fallback content. Destroy a child navigable
given the element.
This check also protects against an `<object>` element being adopted
from a document which has a browsing context to one that doesn't during
the time between the element task being queued and then executed.
This means a browsing context container cannot be ran, meaning browsing
context containers cannot access their parent document and access the
properties and events mentioned in steps 1-11 above, or use
document.{open,write,close} on the parent document.
Another potential avenue of running script via HTML fragment parsing
is via custom elements being in the markup, which need to be
synchronously upgraded. For example:
```
<custom-element></custom-element>
```
However, this is already protected against in the spec:
https://html.spec.whatwg.org/multipage/parsing.html#create-an-element-for-the-token44dd824764/Userland/Libraries/LibWeb/HTML/Parser/HTMLParser.cpp (L643)
> 7. If definition is non-null and the parser was not created as part
of the HTML fragment parsing algorithm, then let will execute
script be true. Otherwise, let it be false.
It is protected against overall by disabling custom elements via
returning `null` for all custom element definition lookups if the
document has no browsing context, which is the case for the temporary
document:
https://html.spec.whatwg.org/multipage/custom-elements.html#look-up-a-custom-element-definition44dd824764/Userland/Libraries/LibWeb/DOM/Document.cpp (L2106-L2108)
> 2. If document's browsing context is null, return null.
This is because the document doesn't have an associated Window, meaning
there will be no associated CustomElementRegistry object.
After running the HTML fragment parser, all of the child nodes are
removed the temporary document and then adopted into the context
element's node document. Skipping the `pre_remove` steps as they are
not relevant in this case, let's first examine Node::remove()'s
potential to execute script, then examine Document::adopt_node() after.
https://dom.spec.whatwg.org/#concept-node-remove44dd824764/Userland/Libraries/LibWeb/DOM/Node.cpp (L534)
1-7. Does not run any script, it just keeps a copy of some data that
will be needed later in the algorithm and directly modifies live
range attributes. However, since this relies on Range objects
containing the temporary document, the Range steps are no-ops.
8. Though this uses the temporary document, it does not contain any
NodeIterator objects as no script should have run, thus this
callback will not be entered. Even if the document _did_ have
associated NodeIterators, NodeIterator::run_pre_removing_steps does
not execute any script.
9-11. Does not run any script, it just keeps a copy of some data that
will be needed later in the algorithm and performs direct tree
mutation to remove the node from the node tree.
12-14. "assign slottables" and step 13 queue mutation observer
microtasks via "signal a slot change". However, since this is
done _after_ running "the end", the "spin the event loop" steps
in that algorithm does not affect this. Remember that queued
microtasks due not execute during this algorithm for the next
few steps.
Sidenote:
Microtasks are supposed to be executed when the JavaScript execution
context stack is empty. Since HTMLParser::parse_html_fragment is only
called from script, the stack will never be empty whilst it is running,
so microtasks will not run until some time after we exit this function.
15. This could potentially run script, let's have a look at the
removal steps we currently have implemented in our engine:
- HTMLIFrameElement::removed_from()
https://html.spec.whatwg.org/multipage/iframe-embed-object.html#the-iframe-element:the-iframe-element-744cf92616e/Userland/Libraries/LibWeb/HTML/HTMLIFrameElement.cpp (L102)
Since browsing context containers cannot create child browsing
contexts (as shown above), this code will do nothing. This will also
hold true when we implement HTMLFrameElement::removed_from() in the
future.
- FormAssociatedElement::removed_from()
44cf92616e/Userland/Libraries/LibWeb/HTML/FormAssociatedElement.h (L36)
This calls `form_node_was_removed` which can then potentially call
`reset_form_owner`. However, `reset_form_owner` only does tree
traversal to find the appropriate form owner and does not execute
any script. After calling `form_node_was_removed` it then calls
`form_associated_element_was_removed`, which is a virtual function
that no one currently overrides, meaning no script is executed.
- HTMLBaseElement::removed_from()
44dd824764/Userland/Libraries/LibWeb/HTML/HTMLBaseElement.cpp (L45)
This will call `Document::update_base_element` to do tree traversal
to find out the new first `<base>` element with an href attribute and
thus does not execute any script.
- HTMLStyleElement::removed_from()
https://html.spec.whatwg.org/multipage/semantics.html#update-a-style-block44dd824764/Userland/Libraries/LibWeb/HTML/HTMLStyleElement.cpp (L49)
This will call `update_a_style_block`, which will parse the `<style>`
element's text content as CSS and create a style sheet from it. This
does not execute any script.
In summary, step 15 does not currently execute any script and ideally
shouldn't in the future when we implement more `removed_from` steps.
16. Does not run any script, just saves a copy of a variable.
17. Queues a "disconnectedCallback" custom elements callback. This will
execute script in the future, but not here.
18. Performs step 15 and 17 in combination for each of the node's
descendants. This will not execute any script.
19. Does not run any script, it performs a requirement of mutation
observers by adding certain things to a list.
20. Does not execute any script, as mutation observer callbacks are
done via microtasks.
21. This will not execute script, as the parent is always the temporary
document in HTML fragment parsing. There is no Document children
changed steps, so this step is a no-op.
We then do layout invalidation which is our own addition, but this also
does not execute any script.
In short, removing a node does not execute any script. It could execute
script in the future, but since this is done by tasks, it will not
execute until we are outside of HTMLParser::parse_html_fragment.
Let's look at adopting a node:
https://dom.spec.whatwg.org/#concept-node-adopt44dd824764/Userland/Libraries/LibWeb/DOM/Document.cpp (L1414)
1. Does not run script, it just keeps a reference to the temporary
document.
2. No-op, we removed the node above.
3.1. Does not execute script, it simply updates all descendants of
the removed node to be in the context element's node document.
3.2. Does not execute script, see node removal step 17.
3.3. This could potentially execute script, let's have a look at the
adopting steps we have implemented in our engine:
- HTMLTemplateElement::adopted_from()
https://html.spec.whatwg.org/multipage/scripting.html#the-template-element:concept-node-adopt-ext44dd824764/Userland/Libraries/LibWeb/HTML/HTMLTemplateElement.cpp (L38)
This simply adopts the `<template>` element's DocumentFragment node
into its inert document. This does not execute any script.
We then have our own addition of adopting NodeIterators over to the
context element's document, but this does not execute any script.
In short, adopting a node does not execute any script.
After adopting the nodes to the context element's document, HTML
fragment parsing is complete and the temporary document is no longer
accessible at all.
Document and element event handlers are also not accessible, even if
the event bubbles. This is simply because the temporary document is not
accessible, so tree traversal, IDL event handler attributes and
EventTarget#addEventListener are not accessible, on the document or any
descendants. Document is also not an Element, so element event handler
attributes do not apply.
In summary, this establishes that HTML fragment parsers should not run
any user script or internal C++ code that relies on things set up by
"the end". This means that the attributes set up and events fired by
"the end" are not observable in this case. This may have not explored
every single possible avenue, but the general assertion should still
hold. However, this assertion is violated by "the end" containing two
unconditional "spin the event loop" invocations and causes issues with
live web content, so we seek to avoid them.
As WebKit, Blink and Gecko have been able to get away with doing fast
path optimizations for HTML fragment parsing which don't setup
navigation timing, run events, etc. it is presumed we are able to get
away with not running "the end" for HTML fragment parsing as well.
WebKit: c69be377e1/Source/WebCore/dom/DocumentFragment.cpp (L90-L98)
Blink: 15444426f9/third_party/blink/renderer/core/editing/serializers/serialization.cc (L681-L702)
Gecko: 6fc2f6d533/dom/base/FragmentOrElement.cpp (L1991-L2002)
Removing the call to "the end" fixes at least a couple of issues:
- Inserting `<img>` elements via innerHTML causes us to spin forever.
This regressed in 2413de7e10
This is because `m_load_event_delayer.clear()` is performed inside an
element task callback. Because of the reasons stated above, this will
never execute. This caused us to spin forever on step 8 of "the end",
which is delaying the load event.
This affected Google Docs and Google Maps, never allowing them to
progress after performing this action. I have also seen it cause a
Scorecard Research `<img>` beacon in a `<noscript>` element inserted
via innerHTML to spin forever. This presumably affects many more
sites as well.
Given that the Window load event is not fired for HTML fragment
parsers, spinning the event loop to delay the load event does not
change anything, meaning this step can be skipped entirely.
- Microtask timing is messed up by the unconditional `spin_until`s on
steps 7 and 8.
"Spin the event loop" causes an unconditional microtask checkpoint:
https://html.spec.whatwg.org/multipage/webappapis.html#spin-the-event-loop44dd824764/Userland/Libraries/LibWeb/HTML/EventLoop/EventLoop.cpp (L54)
> 3. Let old stack be a copy of the JavaScript execution context
stack.
> 4. Empty the JavaScript execution context stack.
> 5. Perform a microtask checkpoint.
> 6.2.1. Replace the JavaScript execution context stack with old
stack.
This broke YouTube with the introduction of custom elements, as
custom elements use microtasks to upgrade elements and call
callbacks. See https://github.com/whatwg/html/issues/8646 for a full
example reduced from YouTube's JavaScript.
Another potential fix for this issue is to remove the above steps
from "spin the event loop". However, since we have another issue with
the use of "spin the event loop", it would be best to just avoid
both calls to it.
Considering all of the above, removing the call to "the end" is the way
forward for HTML fragment parsing, as all of it should be a no-op.
This is done by not simply returning from "the end" if the HTML parser
was created for HTML fragment parsing.
The end.
This ports MouseEvent, UIEvent, WheelEvent, and Event to new String.
They all had a dependency to T::create() in
WebDriverConnection::fire_an_event() and therefore had to be ported in
the same commit.
Setting the `data` of a text node already triggers `children changed`
per spec, so there's no need for an explicit call.
This avoids parsing every HTMLStyleElement sheet twice. :^)
The UTF-8 encoding of U+00A0 (NBSP) is the bytes 0xc2 0xa0. By looping
over the string to escape byte-by-byte, we replace the second byte with
" ", but leave the first byte in the resulting text. This creates
an invalid UTF-8 string, with a lone leading byte.
Because of interdependencies between DOM::Event and UIEvents::MouseEvent
to template function fire_an_event() in WebDriverConnection.cpp, the
commit: 'LibWeb: Make factory methods of UIEvents::MouseEvent fallible'
have been squashed into this commit.
Having an alias function that only wraps another one is silly, and
keeping the more obvious name should flush out more uses of deprecated
strings.
No behavior change.
