Remove doctype specific code

RESOLVED FIXED

Status

()

Core
DOM
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: sicking, Assigned: smaug)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Created attachment 549283 [details] [diff] [review]
Patch v1

Currently in both ReplaceOrInsertBefore and doInsertChildAt we have code that avoids implicitly adopting doctypes, instead the doctype node itself has a bunch of code in BindToTree to adjust its nodeinfo if it's being implicitly adopted.

This is pretty surprising and temporarily caused a regression in bug 648065.

It appears that the only reason we have all this is to allow us to make explicitly adopting doctypes *not* work (implicit does work though).

I talked with Anne and he removed this strange special case from DOM Core.

So here's a patch that simplifies this whole setup. It also removes some code to handle now-deprecated nodetypes from our adopt/import code.

Comment 1

6 years ago
Try run for d57917a61371 is complete.
Detailed breakdown of the results available here:
    http://tbpl.mozilla.org/?tree=Try&rev=d57917a61371
Results:
    success: 23
    warnings: 3
    failure: 1
Total buildrequests: 27
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/sicking@mozilla.com-d57917a61371

Updated

6 years ago
Assignee: nobody → jonas
Status: NEW → ASSIGNED
OS: Mac OS X → All
Hardware: x86 → All
Olli, would you mind taking this since it looked like you were looking at doctypes over in bug 675166.

The patch here works, but needs tests. The only tests it breaks are the ones from the W3C DOM test suite that checks for the old behavior. We should just nuke those and add tests that ensures that adopting/importing doctypes works as it should.
Assignee: jonas → nobody
(Assignee)

Comment 3

6 years ago
I can look at this after bug 675166 is fixed (since I happened to write the patch for that first).
Assignee: nobody → Olli.Pettay
(Assignee)

Comment 4

6 years ago
And also because bug 675166 shouldn't cause any functional changes, but this bug does.
(Assignee)

Updated

6 years ago
Depends on: 675166
(Assignee)

Comment 5

6 years ago
Bug 675166 fixed also this one.
(Assignee)

Updated

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.