Open Bug 1357132 Opened 5 years ago Updated 2 years ago

Do we still need the style attr special-casing in nsGenericHTMLElement::CopyInnerTo?

Categories

(Core :: DOM: Core & HTML, enhancement, P3)

enhancement

Tracking

()

Tracking Status
firefox55 --- affected

People

(Reporter: bzbarsky, Unassigned, NeedInfo)

References

Details

Attachments

(1 file)

This was added in bug 137428 but I'm not sure why it's needed, really.  If it _is_ needed, I'm not sure why we don't need it for SVG/XUL style attributes.

Needs investigation.

It's _possible_ that this is a performance win (due to not having to reparse the style attr), of course.  Need to check.
Flags: needinfo?(bzbarsky)
The answer is that this is not needed.  In fact, it causes an observable bug: when doing importNode between documents with different base URIs, we should be reparsing the style attr (because the base URI changed) and are not.

I filed bug 1357645 on maybe improving the performance here, but I'm not seeing much of a performance impact from removing this thing in the form it's in right now.
Flags: needinfo?(bzbarsky)
Blocks: 1357645
Comment on attachment 8859449 [details] [diff] [review]
Remove the broken style attr special casing in HTML element cloning

Review of attachment 8859449 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM.
Attachment #8859449 - Flags: review?(michael) → review+
This causes a test failure in the content-security-policy/blink-contrib/inline-style-allowed-while-cloning-objects.sub.html WPT test.  As far as I can tell, this test does not match the spec at all, though it does match the implementations in Firefox/Safari/Chrome.  I filed https://github.com/w3c/web-platform-tests/issues/5614 on that and will wait for a response there before landing this, on the off chance that I misunderstood the spec or that the spec is wrong right now.  Would rather not change behavior twice.
Not much happening at the WPT issue referenced in comment 4 or the related spec issue (https://github.com/w3c/webappsec-csp/issues/212) ...

I was going to mark this qf:p1 because it blocks bug 1357645 which is qf:p1 but based on comment 1, I'm thinking we maybe don't need this to be qf:anything so I'll leave it for now.
Priority: -- → P3
Component: DOM → DOM: Core & HTML

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:bzbarsky, could you have a look please?

Flags: needinfo?(bzbarsky)

This is blocked on some of the Google CSP folks actually responding in the spec issues linked above.

Flags: needinfo?(bzbarsky)

Christoph, would you be able to poke the CSP spec here to get it sorted out? See https://github.com/w3c/webappsec-csp/issues/212

Assignee: bzbarsky → nobody
Flags: needinfo?(ckerschb)
You need to log in before you can comment on or make changes to this bug.