Bug 76143 - setAttributeNS should comply with the obscure rules of DOM2, just like createAttributeNS and createElementNS do
Summary: setAttributeNS should comply with the obscure rules of DOM2, just like create...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on: 16833
Blocks: 76198 76579
  Show dependency treegraph
 
Reported: 2012-01-11 19:50 PST by Eric Seidel
Modified: 2012-01-18 21:41 PST (History)
5 users (show)

See Also:


Attachments
Patch (14.17 KB, patch)
2012-01-11 19:52 PST, Eric Seidel
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cq-03 (12.77 MB, application/zip)
2012-01-11 22:41 PST, WebKit Review Bot
no flags Details
Archive of layout-test-results from ec2-cr-linux-03 (13.57 MB, application/zip)
2012-01-12 00:34 PST, WebKit Review Bot
no flags Details
Patch (14.38 KB, patch)
2012-01-18 15:12 PST, Adam Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel 2012-01-11 19:50:50 PST
setAttributeNS should comply with the obscure rules of DOM2, just like createAttributeNS and createElementNS do

http://samples.msdn.microsoft.com/ietestcenter/domcore/showdomcoretest.htm?Exception_Element_setAttributeNS3
http://samples.msdn.microsoft.com/ietestcenter/domcore/showdomcoretest.htm?Exception_Element_setAttributeNS4
http://samples.msdn.microsoft.com/ietestcenter/domcore/showdomcoretest.htm?Exception_Element_setAttributeNS5

Noticed that we didn't implement this oddity. :)

I implemented the createAttributeNS and createElementNS versions of this code for Acid3.

Don't make me do setAttributeNodeNS or getAttributeNodeNS too. :p
Comment 1 Eric Seidel 2012-01-11 19:52:25 PST
Created attachment 122160 [details]
Patch
Comment 2 WebKit Review Bot 2012-01-11 22:41:37 PST
Comment on attachment 122160 [details]
Patch

Rejecting attachment 122160 [details] from commit-queue.

New failing tests:
css2.1/20110323/abspos-containing-block-initial-004b.htm
css2.1/20110323/background-intrinsic-007.htm
css2.1/20110323/background-intrinsic-004.htm
css2.1/20110323/background-intrinsic-006.htm
css2.1/20110323/replaced-intrinsic-004.htm
css2.1/20110323/background-intrinsic-008.htm
css2.1/20110323/background-intrinsic-005.htm
compositing/images/direct-svg-image.html
css2.1/20110323/background-intrinsic-009.htm
css3/images/cross-fade-overflow-position.html
css2.1/20110323/replaced-intrinsic-002.htm
css2.1/20110323/background-intrinsic-002.htm
css2.1/20110323/replaced-intrinsic-005.htm
css2.1/20110323/abspos-containing-block-initial-004d.htm
css2.1/20110323/replaced-intrinsic-003.htm
css2.1/20110323/background-intrinsic-001.htm
http/tests/misc/SVGFont-delayed-load.html
css2.1/20110323/replaced-intrinsic-001.htm
css2.1/20110323/background-intrinsic-003.htm
Full output: http://queues.webkit.org/results/11170689
Comment 3 WebKit Review Bot 2012-01-11 22:41:58 PST
Created attachment 122173 [details]
Archive of layout-test-results from ec2-cq-03

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: ec2-cq-03  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 4 WebKit Review Bot 2012-01-12 00:34:17 PST
Comment on attachment 122160 [details]
Patch

Attachment 122160 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11170723

New failing tests:
css2.1/20110323/replaced-intrinsic-005.htm
css2.1/20110323/background-intrinsic-004.htm
css2.1/20110323/background-intrinsic-006.htm
css2.1/20110323/replaced-intrinsic-004.htm
css2.1/20110323/background-intrinsic-008.htm
css2.1/20110323/background-intrinsic-005.htm
compositing/images/direct-svg-image.html
css2.1/20110323/background-intrinsic-009.htm
css3/images/cross-fade-overflow-position.html
css2.1/20110323/replaced-intrinsic-002.htm
css2.1/20110323/background-intrinsic-002.htm
css2.1/20110323/background-intrinsic-007.htm
css2.1/20110323/replaced-intrinsic-003.htm
css2.1/20110323/background-intrinsic-001.htm
http/tests/misc/SVGFont-delayed-load.html
css2.1/20110323/replaced-intrinsic-001.htm
css2.1/20110323/background-intrinsic-003.htm
Comment 5 WebKit Review Bot 2012-01-12 00:34:40 PST
Created attachment 122180 [details]
Archive of layout-test-results from ec2-cr-linux-03

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-03  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 6 Adam Barth 2012-01-18 11:19:01 PST
Assigning to investigate test failures.
Comment 7 Eric Seidel 2012-01-18 13:54:51 PST
Sorry.  I will also take care of this tomorrow if you don't fix it today.
Comment 8 Adam Barth 2012-01-18 13:56:00 PST
> Sorry.  I will also take care of this tomorrow if you don't fix it today.

I've got it in the debugger.  I think it's a subtle issue with integrating with the XML parser.
Comment 9 Eric Seidel 2012-01-18 14:06:42 PST
I expect that the XML parser is using this DOM API when it really wants to use some parser API.  It may also be using the "wrong" namespaces or prefixes.
Comment 10 Adam Barth 2012-01-18 14:17:34 PST
The issue seems to be that we're not handling default namespaces correctly.
Comment 11 Adam Barth 2012-01-18 15:12:13 PST
Created attachment 123006 [details]
Patch
Comment 12 Eric Seidel 2012-01-18 15:16:29 PST
Comment on attachment 123006 [details]
Patch

createAttributeNS will have this same trouble too.
Comment 13 Eric Seidel 2012-01-18 15:16:44 PST
Comment on attachment 123006 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=123006&action=review

> LayoutTests/fast/dom/Element/script-tests/setAttributeNS-namespace-err.js:127
> +// Moz throws a "Not enough arguments" exception in these, we don't:

We probably should. :)
Comment 14 Adam Barth 2012-01-18 15:17:53 PST
Eric points out that createAttributeNS has a similar problem: https://bugs.webkit.org/show_bug.cgi?id=76579
Comment 15 Adam Barth 2012-01-18 15:18:26 PST
Comment on attachment 123006 [details]
Patch

I'm switching this to be reviewed by me since the ChangeLog says it's authored by Eric.
Comment 16 WebKit Review Bot 2012-01-18 21:40:37 PST
Comment on attachment 123006 [details]
Patch

Clearing flags on attachment: 123006

Committed r105379: <http://trac.webkit.org/changeset/105379>
Comment 17 WebKit Review Bot 2012-01-18 21:41:28 PST
All reviewed patches have been landed.  Closing bug.