| Summary: | Remove ENTITY_REFERENCE_NODE from WebCore | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||||||||
| Component: | New Bugs | Assignee: | Eric Seidel (no email) <eric> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | abarth, adamk, andersca, ap, cmarcelo, darin, dglazkov, fishd, gustavo, gyuyoung.kim, haraken, jamesr, japhet, jochen, mifenton, ojan, rakuco, tkent+wkapi, vestbo, webkit.review.bot, xan.lopez | ||||||||||||
| Priority: | P2 | Keywords: | WebExposed | ||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Bug Depends on: | |||||||||||||||
| Bug Blocks: | 74670 | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Eric Seidel (no email)
2012-07-14 01:31:27 PDT
Created attachment 152416 [details]
wip
This will cause a zillion w3c tests to fail. It's also unclear to me exactly what balance to strike. Currently I'm removing all mentions of EntityReference, except for Node.ENTITY_REFERENCE_NODE in JavaScript (it felt odd to remove the constant). Other things which I removed, but we might not chose to: window.EntityReference document.createEntityReference WebKit API i removed: WebNode::EntityReferenceNode (chromium) @DOMEntityReference (Mac API) -[DOMDocument createEntityReference] (Mac API) I also removed the Windows DOM bindings, but since those weren't even implemented, I can't imagine that matters. To give context: it was *only* possible to create EntityReference Nodes inside an XML document, using the javascxript document.createEntityReference call. In HTML, that would just throw an exception. And the parser would never create these nodes. This ended up simplifying a whole bunch of Node and Range code. :) I'm interested in thoughts from the Mac API and Chromium API folks. Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI. Please wait for approval from timothy@apple.com (or another member of the Apple Safari Team) before submitting because this patch contains changes to the Apple Mac WebKit.framework public API. Comment on attachment 152416 [details] wip Attachment 152416 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/13243383 Comment on attachment 152416 [details] wip Attachment 152416 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13235414 Comment on attachment 152416 [details] wip Attachment 152416 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13221982 (In reply to comment #2) > This will cause a zillion w3c tests to fail. It's also unclear to me exactly what balance to strike. Yes, this is expected. We just need to rebaseline them. All of your changes look correct to me. I don't know about the Mac API bits, but otherwise, if the ChangeLog had a description and a link to the DOM4 spec, I'd be ready to r+. > This ended up simplifying a whole bunch of Node and Range code. :) That's the idea! As you can imagine, it simplifies the DOM spec a lot as well. Also, looks like you have compile errors on some platforms. Looking at the chromium case, I think you just need to remove EntityReference.idl from a few more places. http://code.google.com/p/chromium/source/search?q=EntityReference.idl&origq=EntityReference.idl&btnG=Search+Trunk Created attachment 152627 [details]
still wip
Comment on attachment 152627 [details] still wip Attachment 152627 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13265145 New failing tests: dom/xhtml/level1/core/hc_nodevalue03.xhtml dom/xhtml/level1/core/hc_attrgetvalue2.xhtml dom/xhtml/level3/core/noderemovechild13.xhtml dom/xhtml/level3/core/documentadoptnode16.xhtml dom/xhtml/level3/core/nodecomparedocumentposition28.xhtml dom/xhtml/level3/core/nodereplacechild19.xhtml dom/xhtml/level3/core/nodecomparedocumentposition27.xhtml dom/html/level1/core/documentinvalidcharacterexceptioncreateentref.html dom/svg/level3/xpath/XPathEvaluator_evaluate_NOT_SUPPORTED_ERR.svg dom/xhtml/level3/core/noderemovechild14.xhtml dom/xhtml/level3/core/documentadoptnode06.xhtml dom/xhtml/level3/core/noderemovechild12.xhtml dom/xhtml/level3/core/nodeinsertbefore15.xhtml dom/svg/level3/xpath/XPathExpression_evaluate_NOT_SUPPORTED_ERR.svg dom/html/level1/core/hc_attrgetvalue2.html dom/html/level1/core/documentinvalidcharacterexceptioncreateentref1.html dom/xhtml/level3/core/nodeinsertbefore11.xhtml dom/xhtml/level3/core/nodereplacechild18.xhtml dom/xhtml/level3/core/nodecomparedocumentposition29.xhtml css3/filters/blur-filter-page-scroll-self.html dom/xhtml/level3/core/nodecomparedocumentposition26.xhtml dom/xhtml/level3/core/noderemovechild15.xhtml dom/xhtml/level3/core/nodereplacechild22.xhtml dom/html/level1/core/hc_nodevalue03.html dom/xhtml/level1/core/documentinvalidcharacterexceptioncreateentref1.xhtml dom/xhtml/level3/core/documentrenamenode27.xhtml dom/xhtml/level3/core/nodegettextcontent17.xhtml dom/xhtml/level1/core/documentinvalidcharacterexceptioncreateentref.xhtml Created attachment 152669 [details]
Archive of layout-test-results from gce-cr-linux-08
The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-08 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Created attachment 152802 [details]
Patch
Comment on attachment 152802 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152802&action=review r=me assuming you make the test changes and all the EWS bots go green. Thanks for doing this! > Source/WebKit/chromium/public/WebNode.h:77 > + // EntityReferenceNodes are deprecated and impossible possible to create in WebKit. s/possible // > LayoutTests/fast/dom/Window/get-set-properties-expected.txt:38 > +*** FAIL: canGet('EntityReference') should be 'true' but instead is false. *** Can you just delete this line from the test? > LayoutTests/fast/dom/dom-constructors-expected.txt:15 > +FAIL TryAllocate('EntityReference') should be exception. Was no constructor. ditto > LayoutTests/fast/dom/wrapper-classes-expected.txt:52 > +FAIL jsWrapperClass(xmlDocument.createEntityReference()) should be EntityReference. Threw exception TypeError: 'undefined' is not a function (evaluating 'xmlDocument.createEntityReference()') > +FAIL jsWrapperClass(xmlDocument.createEntityReference().__proto__) should be EntityReferencePrototype. Threw exception TypeError: 'undefined' is not a function (evaluating 'xmlDocument.createEntityReference()') > +FAIL jsWrapperClass(xmlDocument.createEntityReference().constructor) should be EntityReferenceConstructor. Threw exception TypeError: 'undefined' is not a function (evaluating 'xmlDocument.createEntityReference()') ditto WebKit API change LGTM Comment on attachment 152802 [details] Patch Attachment 152802 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13271303 New failing tests: dom/xhtml/level1/core/hc_nodevalue03.xhtml dom/xhtml/level1/core/hc_attrgetvalue2.xhtml dom/xhtml/level3/core/noderemovechild13.xhtml dom/xhtml/level3/core/documentadoptnode16.xhtml dom/xhtml/level3/core/nodereplacechild33.xhtml dom/xhtml/level3/core/nodecomparedocumentposition28.xhtml dom/xhtml/level3/core/nodereplacechild19.xhtml dom/xhtml/level3/core/nodereplacechild37.xhtml dom/xhtml/level3/core/nodereplacechild35.xhtml dom/xhtml/level3/core/nodecomparedocumentposition27.xhtml dom/svg/level3/xpath/XPathEvaluator_evaluate_NOT_SUPPORTED_ERR.svg dom/xhtml/level3/core/noderemovechild14.xhtml dom/xhtml/level3/core/documentadoptnode06.xhtml dom/xhtml/level3/core/noderemovechild12.xhtml dom/xhtml/level3/core/nodereplacechild34.xhtml dom/xhtml/level3/core/nodeinsertbefore15.xhtml dom/svg/level3/xpath/XPathExpression_evaluate_NOT_SUPPORTED_ERR.svg dom/xhtml/level3/core/nodereplacechild36.xhtml dom/xhtml/level3/core/nodeinsertbefore11.xhtml dom/xhtml/level3/core/nodereplacechild18.xhtml dom/xhtml/level3/core/nodecomparedocumentposition29.xhtml dom/xhtml/level3/core/nodecomparedocumentposition26.xhtml dom/xhtml/level3/core/noderemovechild15.xhtml dom/xhtml/level3/core/nodereplacechild30.xhtml dom/xhtml/level3/core/nodereplacechild22.xhtml dom/xhtml/level3/core/documentrenamenode27.xhtml dom/xhtml/level3/core/nodegettextcontent17.xhtml dom/xhtml/level3/core/nodereplacechild23.xhtml Created attachment 152827 [details]
Archive of layout-test-results from gce-cr-linux-02
The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-02 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
I don't think that we're allowed to remove Objective C APIs even if we're confident that impact would be negligible, are we? Please don't land without approval from Tim Hatcher or Darin Adler. I'm happy to ignore the Obj-c parts of the change, and leave the dead objects in if that would be better for Mac. Tim's been CC'd since the start. I welcome any and all input from deciders at Apple. Yes, Alexey is right. The ObjC API needs to stay. It can be a dead object with the same header. Comment on attachment 152802 [details]
Patch
Marking r- per the above.
Looks like this has been done now. |