Issue #6294 describes an edge case where the browser crash if the same
module is loaded three times in a document, but all attempts fail.
Failure scenario:
1. Module load 1 set the state to "Fetching"
2. Module load 2 registers a callback to `on_complete` since the
current state is "Fetching"
3. Module load 1 finish with a failure, invoking the callback for load
number 2
4. Module load 3 cause a crash. The state is neither "Fetching" or
"ModuleScript", so we'll reset the state to "Fetching". This invokes
the callback for module load 2 again, now with an unexpected state
which will cause an assert violation.
Proposed fix is to remove the condition that invokes `on_complete`
immediately for successfully loaded modules only, the callback should
be invoked regardless of whether the fetch succeeded or failed.
This reveals a separate bug in HTMLScriptElement, where
`mark_as_ready()` can be invoked before
`m_steps_to_run_when_the_result_is_ready` is assigned.
This appears to be a spec bug, reported as
https://github.com/whatwg/html/issues/12073 and addressed by delaying
the callback by a task, similar to the issue was resolved for inline
scripts.
This allows us to use these methods from `SVGAElement` without
inheriting `HTMLHyperlinkElementUtils`, which we can't do for
`SVGAElement` due to a naming conflict with the `href()` method in
`SVGURIReferenceMixin`.
Previously, when creating a policy container from a fetch response, the
Referrer-Policy HTTP header was not being parsed. This meant documents
loaded with a Referrer-Policy header would ignore the policy and use the
default.
This comment at the end of LibWeb/HTML/Focus.cpp:
```
// What? It already doesn't have system focus, what possible
// platform-specific conventions are there?
```
Originally followed and referred to this FIXME on line 319:
```
// FIXME: Otherwise, apply any relevant platform-specific conventions
// for removing system focus from topDocument's
// browsing context, and run the focus update steps with old chain,
// an empty list, and null respectively.
```
During the course of #7233, it was accidentally moved and attached
to a different context, following this comment below on line 325:
```
// NOTE: The unfocusing steps do not always result in the focus
// changing, even when applied to the currently focused
// area of a top-level traversable. For example, if the currently
// focused area of a top-level traversable is a
// viewport, then it will usually keep its focus regardless until
// another focusable area is explicitly focused
// with the focusing steps.
```
Rather than move it to the correct place and become its git blame
villain in the process, I once more seek to remove it.
This was not a specification issue, a non-string value will just
not match against any of the names and will fall back to Error,
so we should avoid the call of ToString here.
For XHTML documents, resolve named character entities (e.g., )
using the HTML entity table via a getEntity SAX callback. This avoids
parsing a large embedded DTD on every document and matches the approach
used by Blink and WebKit.
This also removes the now-unused DTD infrastructure:
- Remove resolve_external_resource callback from Parser::Options
- Remove resolve_xml_resource() function and its ~60KB embedded DTD
- Remove all call sites passing the unused callback
Model "viewport focus" with Document::focused_area == nullptr.
Focus.cpp:
When a blur occurs, remove the document entry from the old chain
before running the focusing steps. This ensures the document atop the
new chain is not discarded. Focus::run_focus_update_steps will then
set focused_area to nullptr, indicating viewport focus.
EventHandler.cpp:
Split hit testing in handle_mousedown. Use an exact hit test for
focus/blur decisions, and a subsequent cursor hit test for
selection/caret.
A version of this was added in a610639119
and reverted in 70671b4c11. The bugs
there (confusing scroll-to-position and scroll-by-delta, and not having
an execution context in some cases) have been fixed in this version.
A lot of our scrolling code is quite old, and doesn't match the spec,
but does use some similar names. This is quite confusing. In particular
`perform_scroll_of_viewport()` is not the same as the spec algorithm.
That algorithm is actually almost implemented in
`scroll_viewport_by_delta()`.
To clarify things, this commit makes a few changes:
- Rename perform_scroll_of_viewport() to
perform_scroll_of_viewport_scrolling_box(). This is a better match
for how we use this method, even if it's not actually a match for the
algorithm. (:yakbait:)
- Move `scroll_viewport_by_delta()`'s code into a new
`perform_a_scroll_of_the_viewport()` method, and make it take a
position like it should. `scroll_viewport_by_delta()` now calls it
with a calculated position.
I've avoided reusing the original `perform_scroll_of_viewport()` name to
avoid accidents.
When a "length-tracking" TypedArray/DataView is postMessage'd, the
view seen by the recipient should still be "length-tracking". However,
this wasn't the case, because the actual length was serialized, as
opposed to the JS::ByteLength, which includes the "auto" state to
signal the presence of this "length-tracking" behavior.
This fixes two subtests in...
https://wpt.live/html/infrastructure/safe-passing-of-structured-data/messagechannel.any.html
This change replaces our LibXML parser with a new implementation that
wraps libxml2's SAX2 API.
The new Parser class uses libxml2's SAX2 callbacks to drive the existing
XML::Listener interface. That preserves backward compatibility with all
existing consumers (XMLDocumentBuilder, DOMParser, etc.).
This adds visit_edges(Cell::Visitor&) methods to various helper structs
that contain GC pointers, and makes sure they are called from owning
GC-heap-allocated objects as needed.
These were found by our Clang plugin after expanding its capabilities.
The added rules will be enforced by CI going forward.
In the case that initMessageEvent is called (even with the same
ports), we should return a new array object. This fixes an issue
where the cached m_ports_array was not cleared, causing the
attribute to return the old object identity.
This was causing GC-related crashes on various websites, most
prominently on any site that contains embedded YouTube videos. The issue
can be reproduced by going to any YouTube video, using the _Share_
button below it and pasting the embed code into an empty HTML file and
loading it through localhost.
This is technically a regression from
89dbdd3411 in that the problem became
visible with that commit. However, there is nothing wrong with the
commit by itself. It just happens that `Origin::is_same_origin_domain()`
prior to that commit was completely bogus and would mistakenly return
true in almost all cases, so the cross-origin code paths were not
exercised.
I am uncertain how to make a automatic test case for this problem, given
the nature of it being GC- and cross-origin-related. So there is no
regression test included in this commit.
It is not guaranteed that inherited classes have the same address as the
base of the derived class. In that case a reinterpret cast leads to
undefined behavior. This occured on msvc ABI. See:
https://godbolt.org/z/jGeEW3c48
Co-authored-by: ayeteadoe <ayeteadoe@gmail.com>
