Bug 19978 - GTK port always ends up with # at the end of resource URLs (and hence can't load files)
Summary: GTK port always ends up with # at the end of resource URLs (and hence can't l...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Blocker
Assignee: Nobody
URL:
Keywords:
: 19993 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-07-10 14:21 PDT by Jacob Refstrup
Modified: 2008-07-11 15:31 PDT (History)
2 users (show)

See Also:

Attachments
KURL::clearRef added and used to fix situation (2.14 KB, patch)
2008-07-10 14:22 PDT, Jacob Refstrup 		no flags Details | Formatted Diff | Diff
KURL::clearRef added and used to fix situation (2.14 KB, patch)
2008-07-10 14:27 PDT, Jacob Refstrup 		darin: review+
Details | Formatted Diff | Diff
Final patch. (2.16 KB, patch)
2008-07-10 16:30 PDT, Jacob Refstrup 		no flags Details | Formatted Diff | Diff
Final patch (fixed against TOT) (1.68 KB, patch)
2008-07-10 21:56 PDT, Jacob Refstrup 		darin: review+
Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jacob Refstrup 2008-07-10 14:21:31 PDT 
r35040 changed KURL.cpp in setRef() (line 708) testing for isNull() rather than isEmpty() caused a problem on GTK where called setRef("") which ends up being not being null (but would have been empty). This currently causes all file:// references to have added a '#' at the end of the URL -- which means file:// URLs won't load.
Comment 1 Jacob Refstrup 2008-07-10 14:22:24 PDT 
Created attachment 22205 [details]
KURL::clearRef added and used to fix situation

Per Darin's suggestion added a clearRef() to KURL and called this from WebCore/platform/network/curl/ResourceHandleManager.cpp:558.
Comment 2 Jacob Refstrup 2008-07-10 14:27:15 PDT 
Created attachment 22206 [details]
KURL::clearRef added and used to fix situation

Per Darin's suggestion added a clearRef() to KURL and called this from
WebCore/platform/network/curl/ResourceHandleManager.cpp:558.

(this time with a request for a review)
Comment 3 Darin Adler 2008-07-10 15:45:06 PDT 
Comment on attachment 22206 [details]
KURL::clearRef added and used to fix situation

The ChangeLog has tab characters in it. That will make it impossible to check in as is. Otherwise, this looks good.

We should take a look at other callers of setRef and see if any should be changed to use this new function. Perhaps the function should be called removeRef instead of clearRef.

I think this patch is good enough as-is that I will say r=me
Comment 4 Jacob Refstrup 2008-07-10 16:30:56 PDT 
Created attachment 22229 [details]
Final patch.

Renamed to removeRef(); removed tabs. Will need to follow up on AccessibilityRenderObject.cpp seperately.
Comment 5 Mark Rowe (bdash) 2008-07-10 16:32:44 PDT 
*** Bug 19993 has been marked as a duplicate of this bug. ***
Comment 6 Mark Rowe (bdash) 2008-07-10 21:16:53 PDT 
Can you please refresh the patch against TOT.  A change in <http://trac.webkit.org/changeset/35111> added a removeRef method to KURL to address a similar issue inside AccessibilityRenderObject.
Comment 7 Jacob Refstrup 2008-07-10 21:56:28 PDT 
Created attachment 22243 [details]
Final patch (fixed against TOT)

Patched fixed against merge conflict caused by another person adding removeRef(); resolved duplicate and chose to use more streamlined version of removeRef().
Comment 8 Jacob Refstrup 2008-07-10 21:57:30 PDT 
Fixed against TOT.
Comment 9 Darin Adler 2008-07-10 22:15:54 PDT 
Comment on attachment 22243 [details]
Final patch (fixed against TOT)

r=me
Comment 10 Jacob Refstrup 2008-07-11 14:28:18 PDT 
Updated severity to "blocker" as gtk depends on this fix.
Comment 11 Jan Alonzo 2008-07-11 15:31:00 PDT 
Thanks for the patch (and Darin for the review). Landed in r35137