Some of `<svg>`'s children are not exported properly to the clipboard
Categories
(Core :: DOM: Serializers, defect, P2)
Tracking
()
People
(Reporter: mbrodesser-Igalia, Unassigned)
References
(Depends on 1 open bug, Blocks 2 open bugs, Regression)
Details
(Keywords: parity-chrome, regression)
Attachments
(2 files)
STR:
-
Open
data:text/html,<body onload="onLoad()">X<svg width="10" height="10"><g><path d="M-8-6h24v24H-8z" /></g></svg>Y<script>function onLoad() { console.log("onLoad!"); const b = document.querySelector('body'); document.getSelection().selectAllChildren(b); document.addEventListener('selectionchange', () => console.log(document.getSelection().rangeCount)); console.log(document.getSelection());}</script> -
Tap Ctrl+c (or Ctrl+a followed by Ctrl+c).
Expected:
the clipboard's "text/html" flavor contains the <svg> markup including the <path>.
Actual:
<path> is missing.
Exporting a <svg><circle> (see 1569211#c0) doesn't work either.
|
Reporter | |
Comment 1•4 years ago
|
||
The frame of the SVGPathElement returns false when IsSelectable is called from nsDocumentEncoder. That's why <path> isn't exported.
|
|
Reporter | |
Comment 2•4 years ago
|
||
It seems nsIFrame::IsSelectable including its return type, bool, doesn't make sense. Whether a content element is selectable depends on the selection of its context. From the css spec:
"none
The UA must not allow selections to be started in this element.
A selection started outside of this element must not end in this element. If the user attempts to create such a selection, the UA must instead end the selection range at the element boundary."
However, where the selection starts and ends isn't passed to nsIFrame::IsSelectable. Presumably, it should receive that information as additional arguments, so something like: aSelectionStartsOutsideOfFrame, aSelectionEndsOutsideOfFrame. However, not being familiar with this code, I might miss other constraints. For serializing content, this seems the right approach.
Edit: perhaps the nsIFrame instance is already aware of the context. I'm not sure. Then, adding additional parameters is unnecessary.
:emilio: do you know?
|
|
Reporter | |
Comment 3•4 years ago
|
||
The style obtained here seems to be set somewhere in ServoComputedData which is apparently generated from Rust code.
Why do we care about selectability for serialization? Even if the reason makes sense for HTML, does it makes sense for SVG?
|
||
Comment 5•4 years ago
|
||
So, the frame tree does have a bunch of context, yes. But I don't think IsSelectable is wrong necessarily.
IsSelectable fundamentally looks at the value of user-select. This is set to none for a bunch of SVG elements here. Maybe it shouldn't be, per that comment.
Generally, something like this:
<p>Should <span style="user-select: none">this text</span> be copied when copy-pasting the whole paragraph?</p>
I think our behavior in such a test-case is very intentional (and sane, for that matter).
Maybe we should exempt SVG from that particular IsSelectable check for now (since having user-select: none is mostly a hack, looks like).
|
|
Reporter | |
Comment 6•4 years ago
|
||
(In reply to Henri Sivonen (:hsivonen) from comment #4)
Why do we care about selectability for serialization? Even if the reason makes sense for HTML, does it makes sense for SVG?
Maybe not, thanks for bringing that up.
(In reply to Emilio Cobos Álvarez (:emilio) from comment #5)
So, the frame tree does have a bunch of context, yes. But I don't think
IsSelectableis wrong necessarily.
IsSelectablefundamentally looks at the value ofuser-select. This is set tononefor a bunch of SVG elements here. Maybe it shouldn't be, per that comment.
Thanks for the link. Wasn't aware of that file! :-)
Generally, something like this:
<p>Should <span style="user-select: none">this text</span> be copied when copy-pasting the whole paragraph?</p>I think our behavior in such a test-case is very intentional (and sane, for that matter).
Agree.
Maybe we should exempt SVG from that particular IsSelectable check for now (since having
user-select: noneis mostly a hack, looks like).
That's an option. I'll invesigate why the CSS were added for the accessibility-caret.
|
|
Reporter | |
Comment 7•4 years ago
|
||
Maybe we should exempt SVG from that particular IsSelectable check for now (since having user-select: none is mostly a hack, looks like).
It seems getting rid of the hack (which would be the cleanest solution, of course) is hard. The hack still seems necessary nowadays, to avoid important issues like this with the AccessibleCaret.
Therefore, I'll give exempting SVG-elements from the IsSelectable check on serialization a try. However, this also doesn't seem clean; will think more about it.
|
|
Reporter | |
Updated•4 years ago
|
|
|
Reporter | |
Updated•4 years ago
|
|
|
Reporter | |
Updated•4 years ago
|
|
|
Reporter | |
Updated•4 years ago
|
|
||
Updated•4 years ago
|
|
|
Reporter | |
Updated•4 years ago
|
|
|
Reporter | |
Comment 8•4 years ago
|
||
It's possible to HideCarets() in AccessibleCaretManager::OnSelectionChanged when only SVG shapes are selected. This seems to allow to get rid of the hack of styling SVG shapes as user-select: none.
Will have to think more about it and test the relevant cases.
|
||
Comment 9•4 years ago
|
||
I believe this may be the same root cause of the bug I filed here: https://bugzilla.mozilla.org/show_bug.cgi?id=1678876.
Basically, we can't copy HTML which includes <svg> tags, since it breaks the expected "structure". For example, if a <ul> is selected which includes an <svg> as a child, the <ul> tag is actually duplicated on copy.
I've tried to narrow it down, but I'm quite new to hacking on Firefox so I'm not sure. But by the sounds of this bug I'm inclined to think the cause is the same.
|
||
Updated•4 years ago
|
|
|
Reporter | |
Updated•4 years ago
|
|
|
Reporter | |
Updated•4 years ago
|
|
|
Reporter | |
Comment 11•4 years ago
|
||
Unassigning this, because presumably I won't work on this during the coming weeks.
|
||
Updated•4 years ago
|
|
|
Reporter | |
Updated•4 years ago
|
|
|
Reporter | |
Comment 12•4 years ago
|
||
Writing a WPT was not easily possible, hence wrote a Mochitest.
|
|
Reporter | |
Comment 13•4 years ago
|
||
Styling the svg shapes as user-select: none was added in bug
1286882 as a hack to prevent showing the AccessibleCaret when tapping on
such shapes.
That broke serializing/exporting the "text/html" clipboard-flavor.
Consequently, that broke copy-pasting selections containing svg shapes
to contenteditable elements.
TYLin: I'm not sure whether checking for svg elements (instead of only
shapes is too lax). Moreover, the patch isn't finalized yet (some Marionette test is failing),
but I'd like your feedback about the idea.
Depends on D101174
|
|
Reporter | |
Updated•4 years ago
|
|
|
Reporter | |
Updated•4 years ago
|
|
||
Updated•4 years ago
|
Description
•