KURL ref() methods should be fragmentIdentifier() methods. HTML 5 has standardized on the terminology "fragment identifier" for the fragment/ref string after the #. Having ref() be a method causes headaches, being overloaded with the "ref()" of ref pointer fame. I have a cleanup that replaces: ref(), hasRef(), setRef(const String&), removeRef(), equalIgnoringRef() with fragmentIdentifier(), hasFragmentIdentifier(), setFragmentIdentifier(const String&), removeFragmentIdentifier(), equalIgnoringFragmentIdentifier()
Created attachment 34327 [details] Proposed patch Pretty mechanical change. But there are some very subtle "null versus empty" changes, also, so even if this get's an r+, note that I'm still running all the layout tests on all the platforms I have access to, just to make sure those subtle changes don't break anything, and I'll finish that and land after the weekend.
Comment on attachment 34327 [details] Proposed patch > - const KURL& url = this->url(); > - return url.ref().isEmpty() ? "" : "#" + url.ref(); > + const String& fragmentIdentifier = this->url().fragmentIdentifier(); > + return fragmentIdentifier.isEmpty() ? "" : "#" + fragmentIdentifier; No need for the this-> any more here. r=me
Comment on attachment 34327 [details] Proposed patch You're my personal hero. :) Why do we even need to check "has" before "remove"?
Sending WebCore/ChangeLog Sending WebCore/accessibility/AccessibilityRenderObject.cpp Sending WebCore/bindings/js/JSLocationCustom.cpp Sending WebCore/css/CSSCursorImageValue.cpp Sending WebCore/history/HistoryItem.cpp Sending WebCore/html/HTMLAnchorElement.cpp Sending WebCore/html/HTMLFrameElementBase.cpp Sending WebCore/loader/FrameLoader.cpp Sending WebCore/loader/appcache/ApplicationCache.cpp Sending WebCore/loader/appcache/ApplicationCacheGroup.cpp Sending WebCore/loader/appcache/ApplicationCacheResource.h Sending WebCore/loader/appcache/ApplicationCacheStorage.cpp Sending WebCore/loader/appcache/ManifestParser.cpp Sending WebCore/page/Location.cpp Sending WebCore/page/Page.cpp Sending WebCore/platform/KURL.cpp Sending WebCore/platform/KURL.h Sending WebCore/rendering/RenderPartObject.cpp Sending WebCore/workers/WorkerLocation.cpp Transmitting file data ................... Committed revision 46978. Two platforms I can build and test on are clear, I'll keep a close eye on the bots.
@Eric Seidel DOH, just saw your comment. Good point, there's no reason.
Actually, currently removeFragmentIdentifier() blindly reallocs and reparses, whilst hasFragmentIdentifier() is cheap. There's no reason that removeFragmentIdentifier() shouldn't be a little smarter.
https://bugs.webkit.org/show_bug.cgi?id=28151
(In reply to comment #4) > Two platforms I can build and test on are clear, I'll keep a close eye on the > bots. http://build.webkit.org/builders/GTK%20Linux%20Release/builds/2853/steps/compile-webkit/logs/stdio
Chromium build borked (obviously). Fix coming up.
(In reply to comment #8) > (In reply to comment #4) > > Two platforms I can build and test on are clear, I'll keep a close eye on the > > bots. > > http://build.webkit.org/builders/GTK%20Linux%20Release/builds/2853/steps/compile-webkit/logs/stdio Sending gtk/ChangeLog Sending gtk/webkit/webkitdownload.cpp Transmitting file data .. Committed revision 46980. Hope that's it, I'll keep watching.
Chromium build fix landed as http://trac.webkit.org/changeset/46993.
Sorry for the mild pain, guys. Glad we did this one, though!
no worries :).