This fixes the relevant warnings when running LibJSGCVerifier. Note that
the analysis is only performed over LibJS-adjacent code, but could be
performed over the entire codebase. That will have to wait for a future
commit.
Currently the `<select>` dropdown IPC uses the option value attr to
find which option is selected. This won't work when options don't
have values or when multiple options have the same value. Also the
`SelectItem` contained so weird recursive structures that are
impossible to create with HTML. So I refactored `SelectItem` as a
variant, and gave the options a unique id. The id is send back to
`HTMLSelectElement` so it can find out exactly which option element
is selected.
As defined in: https://w3c.github.io/pointerevents
With the exception of the getCoalescedEvents and getPredictedEvents
APIs.
There are still many other parts of that spec (such as the event
handlers) left to implement, but this does get us at least some of the
way.
Doing multiple `for_each_in_subtree()` passes was kind of a hack. We
can resolve everything in a single pass with a little more control over
the layout process. This also fixes a few minor issues like the sizing
of nested `<g>` elements.
More work is needed here though as this is still fairly ad-hoc.
Note: This does regress `css-namespace-tag-name-selector.html`,
previously SVG text within `<a>` elements would appear. However, this
was only because `for_each_in_subtree()` would blindly look through the
InlineNodes from the unimplemented `SVGAElement`s.
If initial src of an iframe is "about:blank", it does synchronous
navigation that is not supposed to be interleaved by other navigation
or usage of Document.open().
Fixes crashing in navigation on https://twinings.co.uk/
As the parser was trying to directly unwrap an unresolved duration.
Currently we are outputting the wrong results for the serialized
duration, but this is still a step forwards.
Fixes a crash seen on: https://evaparish.com/blog/how-i-edit
A bunch of this is leftover from pre porting over to new AK::String.
For example, for functions which previously took a ByteString const&
now accepting a StringView.
Fixes a bug when session history traversal queue task could be
interrupted by another SHTQ task execution. For example:
1. SHTQ timer callback starts executing a task from the queue.
2. spin_until() is invoked during task execution.
3. SHTQ timer callback starts executing a task from the queue.
...because existance of this method conflicts with the purpose of
having a queue as it allows to start executing next task in the middle
of ongoing task.
For example:
1. SHTQ timer starts executing a task.
2. Task does SHTQ::process().
Another example:
1. SHTQ::process() start executing a task.
2. task does SHTQ::process().
`HTMLIFrameElement::inserted()` does following:
1. Init a new navigable. This step appends a task on session history
traversal queue (SHTQ) that creates a new nested history.
2. Process iframe's attributes
Processing of iframe's attributes might result in synchronous
navigation that fails to get active SHE if SHTQ task that creates
new nested history is not yet completed.
Before this change, a workaround involved forcing the processing of
SHTQ, which was terrible hack because it could occur in the middle of
another SHTQ task.
This change removes the need for "force SHTQ processing" by ensuring
that the processing of iframe's attributes is always executed after
the iframe's navigable nested history has been created.
Workaround spec bug by explicitly carrying information whether
navigation is sync (History api, fragment change) or not.
See for more details https://github.com/whatwg/html/issues/10232
While this is the default for an underlying socket, it doesn't seem good
to have this as the default for our socket wrapper.
This fixes a crash in ladybird when connecting to the python HTTP server
with HTTPS.
If navigation early returns before reaching "finalize a cross document
navigation" then we have to make sure delaying load events is disabled.
See spec issue https://github.com/whatwg/html/issues/10252
Before this change, every CSS @supports rule would keep the containing
JS realm alive via a JS::Handle. This led to a GC reference cycle and
the whole realm leaked.
Since we only need the realm at construction time, we can take it as a
parameter instead, and stop storing it.
...and use HeapFunction instead of SafeFunction for task steps.
Since there is only one EventLoop per process, it lives as a global
handle in the VM custom data.
This makes it much easier to reason about lifetimes of tasks, task
steps, and random stuff captured by them.
This allows you to click on a <img> that has an ismap attribute, and
will result in the navigation URL having the coordinates appended as a
query to the URL.
It's a little bit confusing and awkward that we have `url` _and_
`url_string` here, but let's just fix the typo so that we correctly pass
through the URL with the given suffix (if any).
Currently, nothing is actually passing through this suffix - so it
doesn't fix anything yet, but it becomes relevant in the next commit.
This is often used on login forms, for example, to toggle the visibility
of a password. The site will change the <input> element's type to "text"
to allow the password to show.
No need to force an allocation. This makes a future patch a bit simpler,
where we will have the encoding as a String. With this patch, we won't
have to convert it to a ByteString.
This patch implements the File API spec's supplemental steps for
document's "unloading document cleanup steps" so that we now remove blob
URLs associated with the document's relevant settings object when the
document is being unloaded.
Fixes two realm leaks when running our test suite.
f66d33423b was not sufficient to ensure
document destruction when a child navigable is destroyed. This is
because a navigable was remove from the set of all navigables too early
which led to `Navigable::navigable_with_active_document()` being unable
to find a navigable that is still in the process of destruction.
This change solves that by making all steps of a navigable destruction
to happen in afterAllDestruction callback.
Unfortunately, writing a test to verify document destruction is
challenging because no events are emitted to indicate that it has
happened.
Switching away from SafeFunction immediately backfired here, as we're
dealing with two layers of captures, not one.
Let's do the correct fix, which is to use HeapFunction. This makes the
API and its behavior explicit, and keeps captures alive as long as the
HeapFunction is alive.
Fixes#23819.
This is one of the cases where SafeFunction actually makes sense.
Since every resource load will always either succeed, fail, or time out,
it's okay to use a SafeFunction since we know it will eventually get
destroyed.
Until it does, this allows it to keep any captures alive.
SafeFunction was causing massive GC reference cycles here and leaking
entire realms as a result.
Since we end up storing these reaction steps in a JS::NativeFunction
(which uses JS::HeapFunction internally) there should be no need to
protect the captures with SafeFunction.
This dramatically shrinks our memory footprint while running tests.
See for more details:
https://github.com/whatwg/html/issues/10242
Before this change it only worked because of another bug in
`EventLoop::spin_processing_tasks_with_source_until()`
where we execute tasks regardless of whether they are runnable or not.
Previously, we were accessing the performance through the current
window object. Thus caused a crash when `animate()` was called on an
element within a document with no associated window object. The global
object is now used to access the performance object in places where
a window object is not guaranteed to exist.
Once we have built up a cache, we can use that internally for operations
on the collection, instead of copying over the list of elements every
time.
On a synthentic benchmark of a page with ~500 link elements, this
results in a 45% percent speedup on my machine.
```html
<body>
<ul>
<li><a href="#">Link 1</a></li>
...
<li><a href="#">Link N</a></li>
</ul>
<script>
window.onload = function() {
const startTime = performance.now();
for (let i = 0; i < 1_000_000; ++i) {
const numLinks = document.links.length;
}
const endTime = performance.now();
const timeTaken = endTime - startTime;
console.log(timeTaken);
};
</script>
</body>
</html>
```
This collection has some pretty strange behaviour, particularly with the
IsHTMLDDA slot which is defined in the javascript spec specifically for
this object.
This commit implements pretty much all of this interface, besides from
the custom [[Call]].
There is also no caching over this collection. Since it is a live
collection over the entire document, the performance is never going to
be great, and I am not convinced any speedup for this legacy interface
is worth a massive cache.
These changes do not solve hanging `location.reload()` and
`location.go()` but only align implementation with the latest edits in
the specification.
`WindowProxy-Get-after-detaching-from-browsing-context` test output is
affected because `iframe.remove();` no longer synchronously does
destruction of a document, but queues a task on event loop.
Co-Authored-By: Andrew Kaster <akaster@serenityos.org>
Going via the `ViewportPaintable` missed some steps (in particular
computing clip rects), which meant nested SVGs within SVGs-as-images
were completely clipped.
This adds an IPC for chromes to mute a tab. When muted, we trigger an
internal volume change notification and indicate that the user agent has
overriden the media volume.
It's not safe to capture a local NonnullGCPtr by reference in a closure
that will execute at some arbitrary time in the future when the local is
out of scope.
I saw a suspicious null dereference in
check_if_access_between_two_browsing_contexts_should_be_reported(),
but I have no idea how to reproduce it.
Still, it seems harmless to paper over it with a FIXME debug log.
While waiting for a task that populates a session history entry, we
can't limit the processing of the event loop to the
`NavigationAndTraversal` task source. This is because fetching uses the
`Networking` task source, which also needs to be processed.
Since making a fetch request might take some time, we want to process
everything on the event loop while waiting, to avoid blocking user
interactions.
It is still possible to use `spin_processing_tasks_with_source_until()`
on subsequent steps of `apply_the_history_step()`.
Also modifies test that was flaky.
The current location of the IPC invocation is often too soon. We reach
it before the audio file has completely finished downloading / loading,
thus there isn't an AudioTrack object yet.
Instead, wait until we are actually playing the audio to invoke the IPC.
This is particularly frequent on bandcamp.
This works just like masks, with a few more (spec imposed) limitations.
All the style properties on the contents of a <clipPath> are ignored,
and instead we just paint the "raw geometry" (as all black), then apply
that as an alpha mask to the target element.
If the element has both a `mask` and `clip-path` set, we compute the
masking area as the intersection of the mask rect and clip rect, then
apply the clip mask to the `mask` (producing a combined mask).
Fixes#19648Fixes#23006
This uses the same trick as done for masks in #23554. Each use of an
SVG `<clipPath>` becomes it's own layout subtree rooted at it's user.
This allows each use have it's own layout (which allows supporting
features such as `clipPathUnits`).
Our implementation was errantly matching HTML tags other than the list
specified by the spec. For example, a <meta name=title> tag would be a
match for document.title.
For example, bandcamp will dynamically update its title when audio is
played as follows:
document.title = "▶︎ " + document.title;
And bandcamp also has a <meta name=title> tag. The result was that the
title would become "▶︎ [object HTMLMetaElement]".
Let's not re-invoke the "page did start loading" IPC when the history
state is pushed/replaced. It's a bit misleading (the change does not
actually load the new URL), but also the chromes may do more work than
we want when we change the URL.
Instead, add a new IPC for the history object to invoke.
From https://html.spec.whatwg.org/#list-of-animation-frame-callbacks:
Each target object has a map of animation frame callbacks, which is
an ordered map that must be initially empty, and an animation frame
callback identifier, which is a number that must initially be zero.
Implements the "top layer" concept from "CSS Positioned Layout Module
Level 4" specification.
- The tree builder is modified to ensure that layout nodes created by
top layer elements are children of the viewport.
- Implements missing steps in `showModal()` to add an element top top
layer.
- Implements missing steps in `close()` to remove an element from top
layer.
Further steps could be:
- Add support for `::backdrop` pseudo-element.
- Implement the "inert" concept from HTML spec to block hit-testing
when element from top layer is displayed.
This will allow fine grained control over animation times, which will
allow us to write timing tests that can reliably pass on the much slower
CI machines.
Most browsers have some indicator when audio is playing in a tab, which
makes it easier to find that tab and mute unwanted audio. This adds an
IPC to allow the Ladybird chromes to do something similar.
Although the flex algorithm as specified does say to determine the cross
size of the flex container, this is not how our layout engine works.
The parent formatting context is responsible for sizing its children,
and since that's already happening, we can simply remove the cross
sizing step from FFC.
If a box has definite width and a preferred aspect ratio, we can
determine its height without performing layout. Hence, its height should
also be considered definite. And the other way around for width from
height as well.
This is preparation for the next commit, which will make flex layout
relinquish control of the flex container cross size to the parent
formatting context.
This way we can just leave it alone if the property hasn't changed.
Notably, if the play-state property has been set to 'paused', and then
the user gets the animation with JS and calls .play() on it, it should
start playing despite the play-state property value.
We were off-by-one when returning the result of parsing a quoted string
in Web::Fetch::Infrastructure::collect_an_http_quoted_string. Instead of
backtracking the lexer and consuming the backtracked string, do a simple
substring operation.
Since the introduction of `AbortSignal::any()`, the specification says
`AbortSignal::create_dependent_abort_signal()` should be used where
`AbortSignal::follow` was previously.
We don't want to always invoke the .play() method if an animation is
relevant and had a play state of running, as the play state is
essentially just "paused" or "not paused". We replace that relevancy
check with the inverse of the pause check so we don't constantly call
.play() on animations that are finished.
Also duplicates and moves the temporary context into both blocks, since
most of the time neither condition will be true.
Rather than trying to store a Handle to a WebIDL::BufferSource, let's
look ahead to what the spec wants us to do with this field and get a
copy of the bytes held by the buffer source right away.
This is a fetching AO and is only used by LibWeb in the context of fetch
tasks. Move it to LibWeb with other fetch methods.
The main reason for this is that it requires the use of other LibWeb AOs
such as the forgiving Base64 decoder and MIME sniffing. These AOs aren't
available within LibURL.
Fixes delayed repainting in the following case:
1. Style or layout invalidation triggers html event loop processing.
2. Event loop processing does nothing because there is no rendering
opportunity.
3. Style or layout change won't be reflected until something else
triggers event loop processing
The spec for each of these state:
-> EOF:
This is an eof-in-comment parse error. Emit the current comment
token. Emit an end-of-file token.
We were neglecting to emit the current comment token before emitting an
EOF token. Note the existing EMIT_CURRENT_TOKEN macro was unused.
If box is sized as replaced it still could be anything, not only SVG.
This fixes crashing on https://www.shopify.com/ that was caused by a
missing paintable for a box that has a layout node. This occurred
because the box was not laid out in dimension_box_on_line().
`Node::shadow_including_root()` was missing a null check, which caused
a crash when manipulating a select element, whose option elements were
initially detached.
The HTMLMediaElement, for example, contains spec text which states any
ongoing fetch process must be "stopped". The spec does not indicate how
to do this, so our implementation is rather ad-hoc.
Our current implementation may cause a crash in places that assume one
of the fetch algorithms that we set to null is *not* null. For example:
if (fetch_params.process_response) {
queue_fetch_task([]() {
fetch_params.process_response();
};
}
If the fetch process is stopped after queuing the fetch task, but not
before the fetch task is run, we will crash when running this fetch
algorithm.
We now track queued fetch tasks on the fetch controller. When the fetch
process is stopped, we cancel any such pending task.
It is a little bit awkward maintaining a fetch task ID. Ideally, we
could use the underlying task ID throughout. But we do not have access
to the underlying task nor its ID when the task is running, at which
point we need some ID to remove from the pending task list.
...because "change" event should be dispatched on control even if it
has "display: none" style.
This change fixes selection in labels dropdown on GitHub's "new issue"
page.
Given a selector like `.foo .bar #baz`, we know that elements with
the class names `foo` and `bar` must be present in the ancestor chain of
the candidate element, or the selector cannot match.
By keeping track of the current ancestor chain during style computation,
and which strings are used in tag names and attribute names, we can do
a quick check before evaluating the selector itself, to see if all the
required ancestors are present.
The way this works:
1. CSS::Selector now has a cache of up to 8 strings that must be present
in the ancestor chain of a matching element. Note that we actually
store string *hashes*, not the strings themselves.
2. When Document performs a recursive style update, we now push and pop
elements to the ancestor chain stack as they are entered and exited.
3. When entering/exiting an ancestor, StyleComputer collects all the
relevant string hashes from that ancestor element and updates a
counting bloom filter.
4. Before evaluating a selector, we first check if any of the hashes
required by the selector are definitely missing from the ancestor
filter. If so, it cannot be a match, and we reject it immediately.
5. Otherwise, we carry on and evaluate the selector as usual.
I originally tried doing this with a HashMap, but we ended up losing
a huge chunk of the time saved to HashMap instead. As it turns out,
a simple counting bloom filter is way better at handling this.
The cost is a flat 8KB per StyleComputer, and since it's a bloom filter,
false positives are a thing.
This is extremely efficient, and allows us to quickly reject the
majority of selectors on many huge websites.
Some example rejection rates:
- https://amazon.com: 77%
- https://github.com/SerenityOS/serenity: 61%
- https://nytimes.com: 57%
- https://store.steampowered.com: 55%
- https://en.wikipedia.org: 45%
- https://youtube.com: 32%
- https://shopify.com: 25%
This also yields a chunky 37% speedup on StyleBench. :^)
I've seen a crash when trying to verify_cast some block-level box to a
BlockContainer when it's actually something else.
This patch adds a debug log message so we can learn more about it next
time it happens somewhere.
Since we drive painting for SVG-as-image manually anyway, there's no
need for them to say they are "ready to paint", since that just causes
unnecessary extra processing in the HTML event loop.
Instead of invalidating animated style properties whenever
`Document::update_style()` is called, now we only do that when
animations might have actually progressed. We still have to ensure
animated properties are up-to-date in `update_style()` to ensure that
JS methods can access updated style properties.
Before this change, we ran style and layout updates from both event
loop processing and update timers. This could have caused missed resize
observer updates and unnecessary updating of style or layout more than
once before repaint.
Also, we can now be sure unnecessary style or layout updates won't
happen in `EventLoop::spin_processing_tasks_with_source_until()`.
In our implementation of the "apply the history step" algorithm, we
have to spin-wait for the completion of tasks queued on the event loop.
Before this change, we allowed tasks from any source to be executed
while we were waiting. It should not be possible because it allows to
interrupt history step application by anything, including another
history step application.
Fixes https://github.com/SerenityOS/serenity/issues/23598
This isn't required as the StyleComputer will do this when animating,
but this allows the properties to be resolved once instead of on
every animation frame.
Note that we still pass AllowUnresolved::Yes because the properties will
not be resolved if there is no target.
When iterating through a @keyframes rule, it isn't possible to resolve
unresolved style properties since there are no elements. This change
allows those properties to simply pass through this helper function.
These will need to store unresolved styles as well, since they may be
built during parsing of a @keyframes rule. In that case there is no
target element or pseudo-element, and thus the value cannot be resolved.
structured_deserialize_internal() is added to support sub
deserialization from serializable interfaces serialization steps which
needs the ability to pass onto the current position in the deserialized
data.
If an unexpected token is encountered when parsing an SVG attribute it
is now immediately propagated with ErrorOr. Previously, some situations
where an unexpected token was encountered could cause a crash.
Use the new DOM tree version mechanism to allow HTMLCollection to
remember its internal list of elements instead of rebuilding it on
every access.
This avoids thousands of full DOM walks while loading our GitHub repo.
~15% speed-up on jQuery subtests in Speedometer 3.0 :^)
This patch adds a u64 version counter to DOM::Document that increments
whenever the tree structure changes (via node insertion or removal),
or an element attribute is changed somehow.
This will be used as a crude invalidation mechanism for HTMLCollection
to cache its elements.
- Compare only the animated properties
- Clone only the hash map containing animated properties, instead of
the entire StyleProperties.
Reduces `KeyframeEffect::update_style_properties()` from 10% to 3% in
GitHub profiles.