Bug 92310 - Href attribute with javascript protocol is stripped when content is pasted into a XML document
Summary: Href attribute with javascript protocol is stripped when content is pasted in...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
: 92329 (view as bug list)
Depends on: 92307
Blocks: 92298
  Show dependency treegraph
 
Reported: 2012-07-25 15:32 PDT by Ryosuke Niwa
Modified: 2013-01-16 17:27 PST (History)
13 users (show)

See Also:


Attachments
Fixes the bug (13.64 KB, patch)
2012-07-25 20:07 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from gce-cr-linux-08 (596.28 KB, application/zip)
2012-07-25 22:14 PDT, WebKit Review Bot
no flags Details
Archive of layout-test-results from gce-cr-linux-05 (550.23 KB, application/zip)
2012-07-25 23:15 PDT, WebKit Review Bot
no flags Details
Patch (14.42 KB, patch)
2012-07-26 12:30 PDT, Ryosuke Niwa
abarth: review+
webkit-ews: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2012-07-25 15:32:04 PDT
When we paste a HTML content into a XML document, we end up stripping href content attribute altogether instead of emptying it out like we do on HTML documents.
Comment 1 Ryosuke Niwa 2012-07-25 15:33:17 PDT
editing/pasteboard/paste-noscript-xhtml.xhtml already tests behavior but the output is hard to read, so I'm going to fix that first in the bug 92307.
Comment 2 Ryosuke Niwa 2012-07-25 19:23:47 PDT
*** Bug 92329 has been marked as a duplicate of this bug. ***
Comment 3 Ryosuke Niwa 2012-07-25 20:07:10 PDT
Created attachment 154530 [details]
Fixes the bug
Comment 4 Early Warning System Bot 2012-07-25 20:21:39 PDT
Comment on attachment 154530 [details]
Fixes the bug

Attachment 154530 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13361285
Comment 5 WebKit Review Bot 2012-07-25 22:14:25 PDT
Comment on attachment 154530 [details]
Fixes the bug

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

New failing tests:
svg/custom/missing-xlink.svg
Comment 6 WebKit Review Bot 2012-07-25 22:14:31 PDT
Created attachment 154539 [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
Comment 7 WebKit Review Bot 2012-07-25 23:15:41 PDT
Comment on attachment 154530 [details]
Fixes the bug

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

New failing tests:
svg/custom/missing-xlink.svg
Comment 8 WebKit Review Bot 2012-07-25 23:15:47 PDT
Created attachment 154552 [details]
Archive of layout-test-results from gce-cr-linux-05

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-05  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment 9 Adam Barth 2012-07-25 23:16:08 PDT
Comment on attachment 154530 [details]
Fixes the bug

Makes sense.
Comment 10 Ryosuke Niwa 2012-07-26 12:30:45 PDT
Created attachment 154719 [details]
Patch
Comment 11 Adam Barth 2012-07-26 12:36:03 PDT
Comment on attachment 154719 [details]
Patch

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

> Source/WebCore/dom/Element.cpp:787
> -                attribute.setValue(nullAtom);
> +                attribute.setValue(emptyAtom);

This change looks odd.  I guess we're using empty string rather than null when removing attributes?

> Source/WebCore/dom/Element.cpp:1465
> +bool Element::parseAttributeName(QualifiedName& out, const AtomicString& namespaceURI, const AtomicString& qualifiedName, ExceptionCode& ec)

We usually put out parameters at the end.  (I know the editing code is weird in this regard.)
Comment 12 Adam Barth 2012-07-26 12:39:46 PDT
Comment on attachment 154719 [details]
Patch

rniwa convinced me that using "" won't go infinite for <iframe src>, so this is probably ok.  A natural followup would be to share code with XSS auditor since XSS auditor is a bit smarter about blocking attributes.
Comment 13 Early Warning System Bot 2012-07-26 12:58:31 PDT
Comment on attachment 154719 [details]
Patch

Attachment 154719 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13351890
Comment 14 Ryosuke Niwa 2012-07-26 13:23:23 PDT
Committed r123788: <http://trac.webkit.org/changeset/123788>
Comment 15 Radar WebKit Bug Importer 2013-01-16 14:49:45 PST
<rdar://problem/13027931>