| Age | Commit message (Collapse) | Author |
|---|
|
This required moving the CSS::StyleProperty destruct out of line.
|
|
Turns out we create a lot of these, mostly from places that don't return
ErrorOr. The yak stack grows.
|
|
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-fragments
https://github.com/SerenityOS/serenity/blob/44dd8247647474df95137452b3c9cad9b83326be/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-end
https://github.com/SerenityOS/serenity/blob/44dd8247647474df95137452b3c9cad9b83326be/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-navigable
https://github.com/SerenityOS/serenity/blob/44dd8247647474df95137452b3c9cad9b83326be/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-6
https://github.com/SerenityOS/serenity/blob/44dd8247647474df95137452b3c9cad9b83326be/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-task
https://github.com/SerenityOS/serenity/blob/44dd8247647474df95137452b3c9cad9b83326be/Userland/Libraries/LibWeb/HTML/HTMLObjectElement.cpp#L58
https://github.com/SerenityOS/serenity/blob/44dd8247647474df95137452b3c9cad9b83326be/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-token
https://github.com/SerenityOS/serenity/blob/44dd8247647474df95137452b3c9cad9b83326be/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-definition
https://github.com/SerenityOS/serenity/blob/44dd8247647474df95137452b3c9cad9b83326be/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-remove
https://github.com/SerenityOS/serenity/blob/44dd8247647474df95137452b3c9cad9b83326be/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-7
https://github.com/SerenityOS/serenity/blob/44cf92616e59bda951b67cdae78a6361bdd76f7a/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()
https://github.com/SerenityOS/serenity/blob/44cf92616e59bda951b67cdae78a6361bdd76f7a/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()
https://github.com/SerenityOS/serenity/blob/44dd8247647474df95137452b3c9cad9b83326be/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-block
https://github.com/SerenityOS/serenity/blob/44dd8247647474df95137452b3c9cad9b83326be/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-adopt
https://github.com/SerenityOS/serenity/blob/44dd8247647474df95137452b3c9cad9b83326be/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-ext
https://github.com/SerenityOS/serenity/blob/44dd8247647474df95137452b3c9cad9b83326be/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: https://github.com/WebKit/WebKit/blob/c69be377e17c2977681fef9113d13d91b62d1ee4/Source/WebCore/dom/DocumentFragment.cpp#L90-L98
Blink: https://github.com/chromium/chromium/blob/15444426f98a99830338697cce54e686e988815c/third_party/blink/renderer/core/editing/serializers/serialization.cc#L681-L702
Gecko: https://github.com/mozilla/gecko-dev/blob/6fc2f6d5335fb6f70f780b5fea5ed77b0719c3b5/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 https://github.com/SerenityOS/serenity/commit/2413de7e10efda16f6f376b42b204279d7a25906
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-loop
https://github.com/SerenityOS/serenity/blob/44dd8247647474df95137452b3c9cad9b83326be/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.
|
|
|
|
The main missing feature here is form associated custom elements.
|
|
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.
|
|
Let's make it clear that these functions deal with ASCII case only.
|
|
|
|
|
|
|
|
|
|
|
|
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.
|
|
|
|
|
|
Callers that are already in a fallible context will now TRY to allocate
cells. Callers in infallible contexts get a FIXME.
|
|
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.
|
|
DeprecatedFlyString relies heavily on DeprecatedString's StringImpl, so
let's rename it to A) match the name of DeprecatedString, B) write a new
FlyString class that is tied to String.
|
|
At this point, the parser is reliable enough that we don't need to spam
the debug log about minor parsing issues on every websites.
|
|
|
|
|
|
This used to be the other way around. If we just inserted input with
document.write, this would always be true and not allow document.write
to immediately parse its input (given that there's no pending parsing
blocking script)
|
|
This will make it easier to support both string types at the same time
while we convert code, and tracking down remaining uses.
One big exception is Value::to_string() in LibJS, where the name is
dictated by the ToString AO.
|
|
We have a new, improved string type coming up in AK (OOM aware, no null
state), and while it's going to use UTF-8, the name UTF8String is a
mouthful - so let's free up the String name by renaming the existing
class.
Making the old one have an annoying name will hopefully also help with
quick adoption :^)
|
|
This algorithm, and window.applicationCache, was removed from the spec:
https://github.com/whatwg/html/commit/e4330d5
This also adds a spec link and comments to the affected parser method.
|
|
This getter and setter were previously labelled as a "hack" and used to
disable style invalidation on attribute changes during the HTML parsing
phase (as it caused big sites's loading to be slow). These functions
are currently not used, so they can be removed:^)
|
|
This commit adds inline spec comments to the part of the parser that
ends up calling HTMLScriptElement::prepare().
The code is tweaked to match the spec more closely.
|
|
This has been renamed in the spec, so let's do it here too.
|
|
These lambdas were marked mutable as they captured a Ptr wrapper
class by value, which then only returned const-qualified references
to the value they point from the previous const pointer operators.
Nothing is actually mutating in the lambdas state here, and now
that the Ptr operators don't add extra const qualifiers these
can be removed.
|
|
This conversion was missing from the table we copied from the spec
for whatever reason.
|
|
Once we know that the current code point is an ASCII character, we can
just check if it's one of the HTML whitespace characters.
Before this patch, we were using the generic StringView::contains(u32)
path that splats a code point into a StringBuilder and then searches
for it with memmem().
This reduces time spent in the HTML tokenizer from 16% to 6% when
loading the ECMA-262 spec.
|
|
|
|
This patch adds inline spec comments, and then adjusts the code a bit
so it reads more like the spec.
|
|
This prevents a reference cycle between a HTMLParser opened via
document.open() and the document. It was one of many things keeping
some documents alive indefinitely.
|
|
|
|
This is being used for more than just time coarsening now, so let's use
the spec's section title for the name.
|
|
This doesn't belong on the EventLoop at all, as far as I can tell.
|
|
This makes checking if a token is a specific tag O(1) instead of O(n).
|
|
|
|
These classes only needed Window to get at its realm. Pass a realm
directly to construct HTML classes.
|
|
This allows <noscript> elements to display their content as proper HTML
instead of raw text when scripting is disabled.
|
|
|
|
This is roughly on-spec, although I had to invent a simple "aborted"
state for the tokenizer.
|
|
This remained undetected for a long time as HeaderCheck is disabled by
default. This commit makes the following file compile again:
// file: compile_me.cpp
#include <LibWeb/HTML/CrossOrigin/CrossOriginOpenerPolicy.h>
// That's it, this was enough to cause a compilation error.
Likewise for most other files touched by this commit.
|
