svg: Add mock SVGImageElement interface#36975
Merged
jdm merged 1 commit intoservo:mainfrom May 13, 2025
Merged
Conversation
Contributor
Author
|
NOTE: There are some unstable WPT testing results because of HTMLVideoElement media player event handling (no "canplaythrough" event sometimes on my local PC), and I have TIMEOUT results (instead of FAILED) for some tests. cc @xiaochengh |
|
🔨 Triggering try run (#14971404913) for Linux (WPT) |
Contributor
Author
jdm
requested changes
May 12, 2025
Member
jdm
left a comment
jdm
left a comment
There was a problem hiding this comment.
I'm fine with these changes, but I'd like to have a plan for using the new layout data or removing the code that's involved with it.
components/script/dom/attr.rs
Outdated
| owner.will_mutate_attr(self); | ||
| self.swap_value(&mut value); | ||
| if *self.namespace() == ns!() { | ||
| if *self.namespace() == ns!() || |
Member
There was a problem hiding this comment.
Let's move this logic into a helper function so we don't duplicate this check everywhere.
Member
There was a problem hiding this comment.
Contributor
Author
There was a problem hiding this comment.
is_trusted_attribute(namespace, local_name) will be ok?
Member
There was a problem hiding this comment.
How about is_relevant_attribute? Trusted makes me think of Trusted Types.
| fn fetch_image_resource(&self, can_gc: CanGc) { | ||
| // TODO: Process and fetch the image resource (as HTMLImageElement). | ||
| // Reject any resource fetching request immediately. | ||
| self.upcast::<EventTarget>() |
Member
There was a problem hiding this comment.
Let's queue a task for this, rather than doing it synchronously.
Contributor
Author
There was a problem hiding this comment.
Like ?
self.owner_global()
.task_manager()
.dom_manipulation_task_source()
.queue_simple_event(self.upcast(), atom!("error"));
components/script/layout_dom/node.rs
Outdated
| this.svg_data() | ||
| } | ||
|
|
||
| fn svg_image_data(&self) -> Option<SVGImageData> { |
Member
There was a problem hiding this comment.
This isn't actually called anywhere right now. Are you planning to add that in a followup?
Contributor
Author
There was a problem hiding this comment.
Actually no plans to implement it.
I just copy-pasted all stuff from SVGSVGElement to make it the same as much as possible (BTW svg_data also doesn't use anywhere)
Member
There was a problem hiding this comment.
Ok, then let's remove the attribute parsing and new traits.
|
Test results for linux-wpt from try job (#14971404913): Flaky unexpected result (20)
Stable unexpected results that are known to be intermittent (12)
Stable unexpected results (4)
|
|
|
tharkum
force-pushed
the
mock-svgimageelement-interface
branch
from
63edb69 to
01693d2
Compare
Contributor
Author
|
@jdm < Please pay attention what this change unblock one crash (yep!). |
tharkum
force-pushed
the
mock-svgimageelement-interface
branch
from
01693d2 to
295e912
Compare
jdm
approved these changes
May 12, 2025
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Contributor
Author
|
In summary
|
Add mock SVGImageElement interface to fix TIMEOUT WPT tests which are related to ImageBitmap (html/canvas/*). https://svgwg.org/svg2-draft/embedded.html#InterfaceSVGImageElement Rationality of this change to fire event "error" on any attempt to fetch image resource on href attribute change to not block WPT tests execution. Some WPT tests use the legacy namespace attribute "xlink:href", so support for it was added to source code. https://svgwg.org/svg2-draft/linking.html#XLinkHrefAttribute - setAttributeNS("http://www.w3.org/1999/xlink", 'xlink:href', src); Testing: Covered by existed WPT tests - fetch/metadata/generated/svg-image* - html/canvas/element/manual/* - html/dom/idlharness.https.html - html/semantics/embedded-content/the-canvas-element/* - html/webappapis/scripting/events/event-handler-all-global-events.html - mozilla/interfaces.https.html Fixes: servo#35881 Signed-off-by: Andrei Volykhin <[email protected]>
tharkum
force-pushed
the
mock-svgimageelement-interface
branch
from
295e912 to
ed84f16
Compare
Contributor
Author
|
@jdm < Please add "T-linux-wpt" label to have WPT results before any attempt to merge PR. |
|
🔨 Triggering try run (#14984538646) for Linux (WPT) |
|
Test results for linux-wpt from try job (#14984538646): Flaky unexpected result (19)
Stable unexpected results that are known to be intermittent (11)
Stable unexpected results (1)
|
|
|
Contributor
Author
|
WPT test:
@jdm < How you handle it (unstable results)? The same symptoms with No required events sometimes from GStreamer backend. GOOD: BAD: |
Member
|
We can file issues for unstable tests with the test path in the issue name and the I-intermittent label. They will cause the test results to be filtered out. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add mock SVGImageElement interface to fix TIMEOUT WPT tests
which are related to ImageBitmap (html/canvas/*).
https://svgwg.org/svg2-draft/embedded.html#InterfaceSVGImageElement
Rationality of this change to fire event "error" on any attempt to fetch
image resource on href attribute change to not block WPT tests execution.
Some WPT tests use the legacy namespace attribute "xlink:href", so support for it was added to source code.
https://svgwg.org/svg2-draft/linking.html#XLinkHrefAttribute
Testing: Covered by existed WPT tests
Fixes: #35881