Bug 94216 - View source doesn't interpret escape characters in hrefs.
Summary: View source doesn't interpret escape characters in hrefs.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Anthony Berent
URL: http://code.google.com/p/chromium/iss...
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-16 06:32 PDT by Anthony Berent
Modified: 2012-08-17 03:47 PDT (History)
3 users (show)

See Also:


Attachments
Test file demonstrating bug (127 bytes, text/html)
2012-08-16 06:53 PDT, Anthony Berent
no flags Details
Patch (7.44 KB, patch)
2012-08-16 07:39 PDT, Anthony Berent
no flags Details | Formatted Diff | Diff
Patch - handle escape chars in view source hrefs. (7.45 KB, patch)
2012-08-17 02:51 PDT, Anthony Berent
no flags Details | Formatted Diff | Diff
Patch - handle escape chars in view source hrefs. (7.44 KB, patch)
2012-08-17 02:53 PDT, Anthony Berent
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anthony Berent 2012-08-16 06:32:30 PDT
Originally reported against Chromium as issue 46737.

" Chrome Version       : 5.0.375.55 (Official Build 47796) beta

What steps will reproduce the problem?
1. Open the attached testcase
2. hit ctrl-u for view-source
3. click the href

What is the expected result?
New tab should open at test.html?a&b

What happens instead?
Tab opens at test.html?a&b"

This is still an issue in all Webkit based browsers (see the Chromium bug for history.).
Comment 1 Anthony Berent 2012-08-16 06:53:51 PDT
Created attachment 158802 [details]
Test file demonstrating bug
Comment 2 Anthony Berent 2012-08-16 07:39:32 PDT
Created attachment 158817 [details]
Patch
Comment 3 Hans Wennborg 2012-08-16 08:14:10 PDT
Comment on attachment 158817 [details]
Patch

Thanks! This seems to work nicely. I only have a tiny nit below.

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

> Source/WebCore/html/HTMLViewSourceDocument.cpp:265
> +int HTMLViewSourceDocument::addRange(const String& source, int start, int end, const String& className, bool isLink, bool isAnchor, const String & link)

nit: the & should be attached to the type, i.e. "const String& link"
Comment 4 Hans Wennborg 2012-08-16 08:14:40 PDT
Maybe Adam can review this, or point us towards someone who can.
Comment 5 Adam Barth 2012-08-16 09:55:46 PDT
Comment on attachment 158817 [details]
Patch

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

Thanks!  Let's clean up the nits and then we'll be all set.

> Source/WebCore/html/HTMLViewSourceDocument.h:61
> -    int addRange(const String& source, int start, int end, const String& className, bool isLink = false, bool isAnchor = false);
> +    int addRange(const String& source, int start, int end, const String& className, bool isLink = false, bool isAnchor = false, const String& link = "");

nit: const String& link = String()

This avoids a malloc by using a null string rather than an empty string.
Comment 6 Anthony Berent 2012-08-17 02:51:06 PDT
Created attachment 159061 [details]
Patch - handle escape chars in view source hrefs.
Comment 7 Anthony Berent 2012-08-17 02:53:37 PDT
Created attachment 159062 [details]
Patch - handle escape chars in view source hrefs.
Comment 8 WebKit Review Bot 2012-08-17 03:47:04 PDT
Comment on attachment 159062 [details]
Patch - handle escape chars in view source hrefs.

Clearing flags on attachment: 159062

Committed r125878: <http://trac.webkit.org/changeset/125878>
Comment 9 WebKit Review Bot 2012-08-17 03:47:07 PDT
All reviewed patches have been landed.  Closing bug.