WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
44036
Port Chromium's WebEntities to HTMLEntityTable
https://bugs.webkit.org/show_bug.cgi?id=44036
Summary
Port Chromium's WebEntities to HTMLEntityTable
Eric Seidel (no email)
Reported
2010-08-15 12:59:34 PDT
Remove Chromium's broken WebEntities code since it depends on removed WebCore files
Attachments
Patch
(6.70 KB, patch)
2010-08-15 13:07 PDT
,
Eric Seidel (no email)
no flags
Details
Formatted Diff
Diff
Patch
(4.78 KB, patch)
2010-08-15 15:30 PDT
,
Adam Barth
no flags
Details
Formatted Diff
Diff
Patch
(4.79 KB, patch)
2010-08-15 15:55 PDT
,
Adam Barth
no flags
Details
Formatted Diff
Diff
Fix for test_shell_tests
(1.32 KB, patch)
2010-08-16 04:51 PDT
,
Yuta Kitamura
hamaji
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1
2010-08-15 13:07:25 PDT
Created
attachment 64452
[details]
Patch
Eric Seidel (no email)
Comment 2
2010-08-15 13:08:33 PDT
CCing the current webkit gardner.
Mihai Parparita
Comment 3
2010-08-15 13:10:09 PDT
+Yaar, Darin and Johnny, since WebEntities was added by Yaar and reviewed by Darin in
http://trac.webkit.org/changeset/52268
based on code written by Johnny, in case any of them have opinions on how to best re-implement this.
Adam Barth
Comment 4
2010-08-15 13:18:08 PDT
Comment on
attachment 64452
[details]
Patch This patch is pretty lame. We should have someone who knows what this code is for comment on whether this is the right approach.
Eric Seidel (no email)
Comment 5
2010-08-15 13:37:24 PDT
I'm in no rush anywhere. Mostly posting this for the gardner, since the chromium build is broken for anyone who does a clean build.
Darin Fisher (:fishd, Google)
Comment 6
2010-08-15 14:34:01 PDT
This looks like it is going to break chromium's page-save-as code. Can the breaking changes made to WebCore be reverted until this is sorted out properly? The whole point of this code living in WebKit was to allow for both changes to be made _together_.
Adam Barth
Comment 7
2010-08-15 14:40:42 PDT
> This looks like it is going to break chromium's page-save-as code. Can the breaking changes made to WebCore be reverted until this is sorted out properly?
I'd rather not. This change required surgery to all the build systems and updates to a bunch of test expectations.
> The whole point of this code living in WebKit was to allow for both changes to be made _together_.
It seems better to port the code to the new entity representation, as I suggested in
https://bugs.webkit.org/show_bug.cgi?id=43949#c35
The code will still be wrong, but at least it will compile.
Adam Barth
Comment 8
2010-08-15 15:30:49 PDT
Created
attachment 64457
[details]
Patch
WebKit Review Bot
Comment 9
2010-08-15 15:36:23 PDT
Attachment 64457
[details]
did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit/chromium/src/WebEntities.cpp:36: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Adam Barth
Comment 10
2010-08-15 15:55:46 PDT
Created
attachment 64458
[details]
Patch
WebKit Review Bot
Comment 11
2010-08-15 15:59:02 PDT
Attachment 64458
[details]
did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit/chromium/src/WebEntities.cpp:36: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Adam Barth
Comment 12
2010-08-15 17:27:45 PDT
btw, I think this patch is wrong. I probably prints out an extra ";" after each entity.
Eric Seidel (no email)
Comment 13
2010-08-15 17:43:17 PDT
I think I might take a stab at moving the lokup down to HTMLEnitySearch.h/cpp. Then this WebEntity code can have its own blacklist. That ends up being a function call per character... which is a bad idea, but no worse than any of the rest of this. :)
Eric Seidel (no email)
Comment 14
2010-08-15 18:30:18 PDT
Comment on
attachment 64458
[details]
Patch It likely does add an extra ';'. I'm tired of fighting this code. James says this is mostly vestigial.
WebKit Commit Bot
Comment 15
2010-08-15 18:49:18 PDT
Comment on
attachment 64458
[details]
Patch Clearing flags on attachment: 64458 Committed
r65388
: <
http://trac.webkit.org/changeset/65388
>
WebKit Commit Bot
Comment 16
2010-08-15 18:49:23 PDT
All reviewed patches have been landed. Closing bug.
Yuta Kitamura
Comment 17
2010-08-16 04:39:12 PDT
(In reply to
comment #15
)
> (From update of
attachment 64458
[details]
) > Clearing flags on attachment: 64458 > > Committed
r65388
: <
http://trac.webkit.org/changeset/65388
>
r65388
broke Chromium's test_shell_tests (DomSerializerTests.* will timeout). It loops infinitely inside while loop of populateMapFromHTMLEntityTable function (it seems "entry" variable does not change inside the loop). Fix seems easy, so I'll try to fix this...
Yuta Kitamura
Comment 18
2010-08-16 04:51:53 PDT
Created
attachment 64484
[details]
Fix for test_shell_tests
Shinichiro Hamaji
Comment 19
2010-08-16 04:55:30 PDT
Comment on
attachment 64484
[details]
Fix for test_shell_tests rs=me
Shinichiro Hamaji
Comment 20
2010-08-16 04:57:34 PDT
Committed
r65411
: <
http://trac.webkit.org/changeset/65411
>
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