WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
43836
Delete LegacyHTMLDocumentParser and LegacyPreloadScanner
https://bugs.webkit.org/show_bug.cgi?id=43836
Summary
Delete LegacyHTMLDocumentParser and LegacyPreloadScanner
Adam Barth
Reported
2010-08-11 01:16:37 PDT
Delete LegacyHTMLDocumentParser and LegacyPreloadScanner
Attachments
Patch
(173.30 KB, patch)
2010-08-11 01:20 PDT
,
Adam Barth
no flags
Details
Formatted Diff
Diff
Patch
(177.28 KB, patch)
2010-08-11 01:40 PDT
,
Adam Barth
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Adam Barth
Comment 1
2010-08-11 01:20:18 PDT
Created
attachment 64086
[details]
Patch
Adam Barth
Comment 2
2010-08-11 01:40:26 PDT
Created
attachment 64087
[details]
Patch
Eric Seidel (no email)
Comment 3
2010-08-11 09:27:44 PDT
Comment on
attachment 64087
[details]
Patch WebCore/dom/DocumentParser.h:62 + // used to implements it. used to implement My name is Eric Seidel, and I support this message.
Darin Adler
Comment 4
2010-08-11 09:30:30 PDT
At Apple we’re starting to get multiple bug reports about Mac OS X applications and websites that don’t work any more due to differences between the old and new parser. One example is AOL’s AIM client, which malfunctions in a spectacular way. But I’m sure more of these reports are coming as people have more time to test. Once you delete the old parser, we will presumably have to add hacks to the new one to make that old content work. Is that your preferred approach?
Adam Barth
Comment 5
2010-08-11 09:50:26 PDT
> One example is AOL’s AIM client, which malfunctions in a spectacular way. But I’m sure more of these reports are coming as people have more time to test.
Yes. Some of these reports have filtered into bugs.webkit.org as bugs blocking
Bug 41115
. If you're able to share the reports (or sanitized versions of the reports), filing them as blocking
Bug 41115
will make sure they don't get lost.
> Once you delete the old parser, we will presumably have to add hacks to the new one to make that old content work. Is that your preferred approach?
We talked this over with Alexey at some point. We think that's a better approach because maintaining branches in the new parser seems easier then maintaining an entirely separate parser. As an example, Chrome decided to ship the old parser in the upcoming M6 beta because we're not quite done with the new parser yet. When they turned it on, they found that it had already bit rotted to the point of a use-after-free on every document parse due to this change:
http://trac.webkit.org/changeset/62302/trunk/WebCore/html/LegacyHTMLDocumentParser.cpp
Notice the comment at the top of the diff that says "this actually causes us to be deleted." I don't mean to blame anyone by pointing out this patch. It just illustrates the general principle that "if it's not tested, it's wrong."
Adam Barth
Comment 6
2010-08-11 11:31:22 PDT
Comment on
attachment 64087
[details]
Patch Clearing flags on attachment: 64087 Committed
r65167
: <
http://trac.webkit.org/changeset/65167
>
Adam Barth
Comment 7
2010-08-11 11:31:28 PDT
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 8
2010-08-11 11:49:50 PDT
http://trac.webkit.org/changeset/65167
might have broken Chromium Win Release The following changes are on the blame list:
http://trac.webkit.org/changeset/65166
http://trac.webkit.org/changeset/65167
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug