When script is looking up an attribute for layout then it only needs to
care about the attribute value. The `Attr` node itself is not required.
If we want to lazily construct attribute nodes in the future
(https://github.com/servo/servo/issues/36697) then we should use `Attr`
as little as possible.
This also ends up simplifying the code by accident.
Testing: No behaviour change intended, regressions are covered by
existing tests.
Part of https://github.com/servo/servo/issues/36697
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
This change is a rework of #22478, originally authored by @vimpunk.
It adds parsing of CSS in parallel with the main script thread. The
big idea here is that when the transfer of stylesheet bytes is
finished, the actual parsing is pushed to a worker thread from the Stylo
thread pool. This also applies for subsequent loads triggered by
`@import` statements.
The design is quite similar to the previous PR with a few significant
changes:
- Error handling works properly. The `CSSErrorReporter` is a crossbeam
`Sender` and a `PipelineId` so it can be trivially cloned and sent to
the worker thread.
- Generation checking is done both before and after parsing, in order
to both remove the race condition and avoid extra work when the
generations do not match.
- The design is reworked a bit to avoid code duplication, dropping added
lines from 345 to 160.
- Now that `process_response_eof` gives up ownership to the
`FetchResponseListener`, this change avoids all extra copies.
Testing: This shouldn't change observable behavior, so is covered
by existing tests.
Fixes: #20721Closes: #22478
---------
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Co-authored-by: mandreyel <mandreyel@protonmail.com>
The two are semantically equivalent, but `find_map` is more concise.
Testing: Covered by existing tests
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
For some specific cases (one of them see below) where are requirement to
identify the reason of the attribute mutation (by whom/why it was
changed - by cloning, parser or directly), so the following `reason`
parameter was added to `AttributeMutation::Set` option.
Note that for the HTMLMediaElement the internal `muted` state should be
set to true when the element is created and the element has a `muted`
content attribute specified (should be done once on the first attribute
mutation caused by parser or cloning).
See https://html.spec.whatwg.org/multipage/#dom-media-muted
Testing: Improvements in the following tests
-
html/semantics/embedded-content/media-elements/user-interface/muted.html
Signed-off-by: Andrei Volykhin <andrei.volykhin@gmail.com>
Removes some unneeded lints, especially `#[allow(unsafe_code)]`.
Testing: Refactor
Part of: #40383
Signed-off-by: WaterWhisperer <waterwhisperer24@qq.com>
Implements the `currentCSSZoom` readonly attribute on the Element
interface as
[spec](https://drafts.csswg.org/cssom-view/#dom-element-currentcsszoom).
- Adds a new layout query (`CurrentCSSZoomQuery`) that traverses from
the target element up through its ancestors
- Accumulates the product of all `zoom` CSS property values to compute
the effective zoom
- Returns 1.0 for elements that are not being rendered (display: none or
no layout data)
Testing: Updated WPT (removed 4 FAIL expectations from
`idlharness.html.ini`). Behavior tests in Element-currentCSSZoom.html
remain as expected FAIL because the underlying CSS `zoom` property
implementation in Servo does not yet apply zoom values to layout (the
zoom property is parsed but computed values remain 1.0).
Fixes: #40256
Signed-off-by: WaterWhisperer <waterwhisperer24@qq.com>
This implements efficient methods for as_bytes, eq_ascii, is_ascii and
to_jsval.
Tests were added for as_bytes. Additionally, BytesView now has an
internal type to make sure nobody can construct it.
Testing: New unit tests were added and some old ones covered the new
functions.
---------
Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
script: add `CanGc` as argument to
`VirtualMethods.post_connection_steps`
Testing: These changes do not require tests because they are a refactor.
Addresses part of https://github.com/servo/servo/issues/34573
Signed-off-by: Yerkebulan Tulibergenov <yerkebulan@gmail.com>
Servo has a lot of comments like this:
```rust
// https://example-spec.com/#do-the-thing
fn do_the_thing() {}
```
and I keep turning these into doc comments whenever I'm working close to
one of them. Doing so allows me to hover over a function call in an IDE
and open its specification without having to jump to the function
definition first. This change fixes all of these comments at once.
This was done using `find components -name '*.rs' -exec perl -i -0777
-pe 's|^([ \t]*)// (https?://.*)\n\1(fn )|\1/// <$2>\n\1$3|mg' {} +`.
Note that these comments should be doc comments even within trait `impl`
blocks, because rustdoc will use them as fallback documentation when the
method definition on the trait does not have documentation.
Testing: Comments only, no testing required
Preparation for https://github.com/servo/servo/pull/39552
---------
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
script: add CanGc as argument to VirtualMethods::children_changed
Testing: These changes do not require tests because they are a refactor.
Addresses part of https://github.com/servo/servo/issues/34573
Signed-off-by: Yerkebulan Tulibergenov <yerkebulan@gmail.com>
Each shadow root holds a map of slot names to slot elements. Slot
elements dynamically register and unregister themselves at their
respective shadow root when they are bound and unbound from the tree.
Previously, they were doing this too often. When a slot element is
unbound from the tree then it might still be connected to its shadow
root, in case the entire shadow tree is being unbound.
Fixing this requires the slot element to know whether it was in a shadow
tree prior to `bind_to_tree` being called (then it's already registered
and doesn't need to do so again), and whether it will be in a shadow
tree after `unbind_from_tree` is called (then there's no need to
unregister).
Testing: This change adds a new web platform test
Fixes: https://github.com/servo/servo/issues/40242
---------
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
add CanGc as argument to Validatable.satisfies_constraints
Testing: These changes do not require tests because they are a refactor.
Addresses part of https://github.com/servo/servo/issues/34573
Signed-off-by: Yerkebulan Tulibergenov <yerkebulan@gmail.com>
Adds an optional message to be attached to an InvalidCharacterError.
Testing: simple refactor
Fixes: one of the items of #39053
---------
Signed-off-by: Alessandro Vannini <alessandrovnnn@gmail.com>
Input methods are very similar to other kinds of embedder controls such
as file selection boxes, so merge them into the same libservo API. This
simplfiies the API surface a bit.
Testing: This change comes with a new unit test.
---------
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
This implements LazyDOMString (from now on DOMString) as outlined in
https://github.com/servo/servo/issues/39479.
Constructing from a *mut JSString we keep the in a
RootedTraceableBox<Heap<*mut JSString>> and transform
the string into a rust string if necessary via the `make_rust_string`
method.
Methods used in script are implemented on this string. Currently we
transform the string at all times.
But in the future more efficient implementations are possible.
We implement the safety critical sections in a separate module
DOMStringInner which allows simple constructors, `make_rust_string` and
the `bytes` method.
This method returns the new type `EncodedBytes` which contains the
reference to the underlying string in either format.
Testing: WPT tests still seem to work, so this should test this
functionality.
---------
Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
The HTML specification defines the 'width' and 'height' attributes as
dimension attributes for the embedded content and images (img, iframe,
embed, object, video, source, input with image type)
https://html.spec.whatwg.org/multipage/#dimension-attributes and for the
tables (table, col, colgroup, thead, tbody, tfoot, tr, td, th) even
these attributes are marked as obsolete.
And UA are expected to use these 'width' and 'height' attributes as
style presentational hints for the rendering.
The embedded content and images:
https://html.spec.whatwg.org/multipage/#dimRendering
The tables:
https://html.spec.whatwg.org/multipage/#tables-2:the-table-element-4
Added missing 'width' and/or 'height' reflected IDL attributes:
- conformant 'unsigned long' IDL attributes for the 'img, source, video'
(with new macro 'make_dimension_uint*)
- obsolete 'DOMString' IDL attributes for the 'td, th, col, colgroup'
Moved the `fn attribute_affects_presentational_hints()` from Element to
specific HTML and SVG elements with attributes which affects
presentational hints for rendering.
Testing: Improvements in the following tests
- html/dom/idlharness.https.html
- html/dom/reflection-embedded.html
- html/dom/reflection-tabular.html
-
html/rendering/replaced-elements/attributes-for-embedded-content-and-images/picture-aspect-ratio.html
Signed-off-by: Andrei Volykhin <andrei.volykhin@gmail.com>
The investigation in #37745 revealed that `Element::create` spends ~39%
of its runtime in `set_custom_element_state`. For uncustomized built-in
elements, this overhead is unnecessary as according to the spec, they
are equivalent to being initialized in the "uncustomized" state.
Although our rust implemention also initializes the elements in
"uncustomized" state, this particular call to `set_custom_element_state`
can't simply be removed as this call also ensures that the element's
DEFINED state is set to 'true', to match the `:defined` selector, based
on the custom element state.
So, introduce a new method that will only set the
`ElementState::DEFINED` flag by manipulating the state directly and call
it from `Element::create` when creating uncustomized, built-ins.
For more information about the peformance improvements, please refer to
[this comment][1].
[1]: https://github.com/servo/servo/issues/37745#issuecomment-3305477287
Testing: There should be no functional change, so this is covered by WPT
suite.
---------
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Follow the HTML specification and take into account that state changes
of the <image> 'crossorigin' and 'referrerpolicy' content attributes
(not 'crossOrigin' and 'referrerPolicy' IDL attributes) should be
counted as relevant mutations.
See https://html.spec.whatwg.org/multipage/#relevant-mutations
Testing: Improvements in the following tests
- html/dom/reflection-embedded.html
-
html/semantics/embedded-content/the-img-element/relevant-mutations.html
Signed-off-by: Andrei Volykhin <andrei.volykhin@gmail.com>
Calling `scrollIntoView()` on an element within an `<iframe>` will now
scroll scrolling boxes from the parent document(s), as long as they have
the same origin.
Testing: One existing subtest passes, and adding a new test.
Signed-off-by: Oriol Brufau <obrufau@igalia.com>
Co-authored-by: Martin Robinson <mrobinson@igalia.com>
Adding an optional message to be attached to a NotFoundError.
Testing: Just a refactor
Part of #39053
---------
Signed-off-by: Rodion Borovyk <rodion.borovyk@gmail.com>
When an `<iframe>` cannot scroll because the size of the frame is
greater than or
equal to the size of page contents, chain up the keyboard scroll
operation to the parent frame.
Testing: A new Servo-only WPT tests is added, though needs to be
manually
run with `--product servodriver`.
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Co-authored-by: Delan Azabani <dazabani@igalia.com>
Regular shadow roots can never be detached once they are created.
However we were specifically detaching shadow roots from media elements
when they were disconnected. This is actually one of the very few
aspects of shadow roots that predate the implementation work I did
earlier this year.
I'm not sure why we ever did this. Maybe its for efficiency reasons,
because keeping the shadow tree around is not necessary when the media
element is not connected. But I can't imagine this yields any benefits,
especially since you would have to reconstruct the shadow tree when the
media element is re-connected (as is the case in the crash we observe).
For simplicities sake, I have completely removed this functionality.
Doing so ends up simplifying the code quite a bit.
Testing: This change adds a new crashtest
Fixes https://github.com/servo/servo/issues/36722
---------
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
Instead of having every single embedder implement keyboard scrolling,
handle it in script in the default key event handler. This allows
properly targeting the scroll events to their scroll containers as well
as appropriately sizing "page up" and "page down" scroll deltas.
This change means that when you use the keyboard to scroll, the focused
or most recently clicked `<iframe>` or overflow scroll container is
scrolled, rather than the main frame.
In addition, when a particular scroll frame is larger than its content
in the axis of the scroll, the scrolling operation is chained to
the parent (as in other browsers). One exception is for `<iframe>`s,
which will be implemented in a followup change.
Testing: automated tests runnable locally with `mach test-wpt --product
servodriver`
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Co-authored-by: Martin Robinson <mrobinson@igalia.com>
Follow the HTML specification and take into account that the HTML
`<image>/<source>` element inserting/removal steps should only be
counted as relevant mutations for `<image>` element if the parent of the
inclusive ancestor that was inserted/removed is the parent `<picture>`
element.
See <https://html.spec.whatwg.org/multipage/#relevant-mutations>.
Testing: Improvements in the following tests
-
html/semantics/embedded-content/the-img-element/relevant-mutations.html
Signed-off-by: Andrei Volykhin <andrei.volykhin@gmail.com>
This isn't needed as the border box query already takes into account the
containing block chain. Instead, consistently calculate the new
scroll position for a scroller relative to its current scroll offset.
In addition, fix a small bug where the border of a scroll container was
considered part of scrollport.
Testing: A new WPT test is added.
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Co-authored-by: Oriol Brufau <obrufau@igalia.com>
- Expose a couple helpers on `ScrollingBox` that will also be used for
keyboard scrolling.
- Calculate `scrollIntoView` positions using points rather than doing
things by axis components. This greatly reduces the amount of code in
the implementation.
Testing: This is just a refactor so shouldn't change any tests.
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
The xpath resolver is a function provided by the user to resolve
namespace prefixes. Previously, we were ignoring the argument.
Testing: New web platform tests start to pass
Part of https://github.com/servo/servo/issues/34527
---------
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
Creating elements by directly calling their interface constructors leads
to some state not being intialized correctly (see #39285). It is also
not in line with the specifications as many of them refer to the
[`create an element`][1] algorithm when an element needs to be created,
which directly maps to `Element::create` in the script crate.
So, switch all such places where elements are created by script to use
`Element::create`.
[1]: https://dom.spec.whatwg.org/#concept-create-element
Testing: Existing WPT tests.
Fixes: #39285
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
To find scrolling ancestors, we need to walk up the flat tree and only
consider the elements that are in the chain of containing block
ancestors of an element. `scrollParent` now does this so we can use it
to properly implement `scrollIntoView`.
Testing: There are WPT tests for this change.
---------
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Co-authored-by: Oriol Brufau <obrufau@igalia.com>
There is no need to separate the two kinds of scrolling boxes into
`Element` and `Viewport` more than once. This also eliminates a
potentially panicking `unwrap()`.
Testing: This doesn't change behavior and is thus covered by existing
tests.
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
It more logically matches `MediaList`, and it allows us to
call this method with a document. This is required when
parsing Link headers, as they don't have an associated
element, but they do have a document.
Part of #35035
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
This clarifies the units for scrolling:
- `f32` is used for internal Servo scrolling APIs as that is the unit
used in WebRender.
- `f64` is used for the web-exposed scrolling APIs as that is what the
WebIDL code generator gives us.
Conversions are done consistently at the boundaries of the two APIs.
In addition, web-exposed scrolling methods are refactored a bit to more
closely follow the specification text. In addition, specification text
is added to those methods so that it is clearer that we are following
it.
Testing: This should not change behavior and is thus covered by existing
tests.
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
With incremental layout adding a restyle to a node isn't enough to force
its layout to update. We also need to explicitly mark the node as dirty
so that its contents are updated when layout is run. This change makes
this consistent for all node state changes. This might be a bit too
conservative as all node state may not affect layout, but should catch
issues in the future.
Testing: This is very hard to test as it requires moving the mouse over
the
WebView, and the moving it away, and then testing the rendered contents.
This
kind of coordination would be difficult to manage with unit tests.
Fixes: #38989.
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Rework the flow of code in `Element:scroll_into_view_with_options` to
more closely follow the specification. This also simplifies the code a
bit and adds some TODOs about future improvements.
Testing: This should not change behavior and is thus covered by existing
tests.
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Co-authored-by: Oriol Brufau <obrufau@igalia.com>
Adding an optional message to be attached to a SyntaxError. Unblocks
#39050.
The enum definition of Syntax is now `Syntax(Option<String>)`. Future
PRs should probably add more appropriate messages to some of the
`Syntax(None)`s.
Testing: Just a refactor
Fixes: Partially #39053
Signed-off-by: Ashwin Naren <arihant2math@gmail.com>
Parameterize and rename both `Layout::content_box_query` and
`Layout::content_boxes_query` to support the query of rendered padding
area and content area that accounts for transform and scroll. Both of
these query have been misleading for a time since they are using border
box, instead of content box of a Node.
This PR adds a new type `layout_api::BoxAreaType` to be passed from
`ScriptThread` to `LayoutThread` to query the respective area. It is
then used for the query within `IntersectionObserver` to pass several
WPTs.
Testing: Existing WPT Coverage.
---------
Signed-off-by: Jo Steven Novaryo <jo.steven.novaryo@huawei.com>
Additionally, several methods were updated with
spec comments. That's because the "adopt the document
from the element document" step was missing.
By adding these spec comments, I also restructured
some code to avoid duplication of mutation records
and custom element reaction queueing.
Node.textContent doesn't propagate the error yet,
as that method has a lot of separate callers of
elements that wouldn't fail. I will refactor those
in a follow-up PR to keep things manageable.
This implements part of the DOM integration from
https://github.com/whatwg/dom/pull/1268
Part of #36258
---------
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
Signed-off-by: Tim van der Lippe <TimvdLippe@users.noreply.github.com>
This shows up sometimes in code reviews, so it makes sense that tidy
enforces it. `rustfmt` supports this via comment normalization, but it
does many other things and is still an unstable feature (with bugs).
Testing: There are new tidy tests for this change.
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Callers now call `set_attribute` directly, to avoid the trusted types
machinery, as well as skip validation. That's not required by spec as
well.
This implements part of the DOM integration from
https://github.com/whatwg/dom/pull/1268
Part of #36258
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
Instead of making a block for each attribute, use a single block as
described in a `FIXME` comment by @emilio. This also switch to using
`map` and `and_then` in more places to make the code a bit more concise.
Testing: This should not change behavior other than to incraese
efficiency a
bit and is thus covered by existing tests.
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Also make TrustedHTML work the same as TrustedScript by
only taking 1 `&str` to make things easier.
Part of #36258
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
