Bug 11960 – REGRESSION: ASSERTION FAILED: Command-clicking an anchor link

• Component: Page Loading
• Status: RESOLVED FIXED

[David Kilzer (:ddkilzer)]

Command-clicking an anchor link (<a href="#anchor">link</a>) causes an assertion failure in debug builds:

ASSERTION FAILED: URL
(/.../WebKit/WebCore/loader/mac/WebDataProtocol.mm:223 +[WebDataProtocol _webIsDataProtocolURL:])
Segmentation fault

Tested on a locally-built debug build of WebKit r18411 with Safari 2.0.4 (419.3) on Mac OS X 10.4.8 (8L1037).

[David Kilzer (:ddkilzer)]

Created attachment 12006 [details]
Test case

[David Kilzer (:ddkilzer)]

Stack trace from debug build:

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_INVALID_ADDRESS (0x0001) at 0xbbadbeef

Thread 0 Crashed:
0   com.apple.WebCore              	0x0137286f +[WebDataProtocol _webIsDataProtocolURL:] + 65
1   com.apple.WebCore              	0x013732e7 WebCore::DocumentLoader::URLForHistory() const + 87 (DocumentLoaderMac.mm:517)
2   com.apple.WebKit               	0x0032f939 -[WebDataSource(WebInternal) _URLForHistory] + 37 (WebDataSource.mm:343)
3   com.apple.WebKit               	0x00333f22 -[WebFrame(WebInternal) _addBackForwardItemClippedAtTarget:] + 64 (WebFrame.mm:278)
4   com.apple.WebKit               	0x0039cef8 WebFrameLoaderClient::addHistoryItemForFragmentScroll() + 56 (WebFrameLoaderClient.mm:1010)
5   com.apple.WebCore              	0x01374a6e WebCore::FrameLoader::continueFragmentScrollAfterNavigationPolicy(WebCore::ResourceRequest const&, bool) + 192 (FrameLoaderMac.mm:467)
6   com.apple.WebCore              	0x01374b02 WebCore::FrameLoader::callContinueFragmentScrollAfterNavigationPolicy(void*, WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool) + 44 (FrameLoaderMac.mm:442)
7   com.apple.WebCore              	0x01378727 WebCore::PolicyCheck::call(bool) + 109 (FrameLoaderMac.mm:1488)
8   com.apple.WebCore              	0x0137affe WebCore::FrameLoader::continueAfterNavigationPolicy(WebCore::PolicyAction) + 388 (FrameLoaderMac.mm:832)
9   com.apple.WebKit               	0x0039da19 WebFrameLoaderClient::receivedPolicyDecison(WebCore::PolicyAction) + 297 (WebFrameLoaderClient.mm:1175)
10  com.apple.WebKit               	0x0039e50d -[WebFramePolicyListener receivedPolicyDecision:] + 133 (WebFrameLoaderClient.mm:1264)
11  com.apple.WebKit               	0x0039d853 -[WebFramePolicyListener ignore] + 43 (WebFrameLoaderClient.mm:1270)
12  com.apple.Safari               	0x00019854 0x1000 + 100436
13  libobjc.A.dylib                	0x90a59d76 objc_msgSendv + 54
14  com.apple.Foundation           	0x925ff43e -[NSInvocation invoke] + 932
15  com.apple.Foundation           	0x92625433 -[NSInvocation invokeWithTarget:] + 67
16  com.apple.WebKit               	0x00368ae4 -[_WebSafeForwarder forwardInvocation:] + 448 (WebView.mm:1669)
17  com.apple.Foundation           	0x925fe4f4 -[NSObject(NSForwardInvocation) forward::] + 469
18  libobjc.A.dylib                	0x90a59cc1 _objc_msgForward + 49
19  com.apple.WebKit               	0x0039dc5a WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(void (WebCore::FrameLoader::*)(WebCore::PolicyAction), WebCore::NavigationAction const&, WebCore::ResourceRequest const&) + 212 (WebFrameLoaderClient.mm:692)
20  com.apple.WebCore              	0x01379270 WebCore::FrameLoader::checkNavigationPolicy(WebCore::ResourceRequest const&, WebCore::DocumentLoader*, WTF::PassRefPtr<WebCore::FormState>, void (*)(void*, WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool), void*) + 630 (FrameLoaderMac.mm:806)
21  com.apple.WebCore              	0x0137a359 WebCore::FrameLoader::load(WebCore::KURL const&, WebCore::String const&, WebCore::FrameLoadType, WebCore::String const&, WebCore::Event*, WebCore::HTMLFormElement*, WTF::HashMap<WebCore::String, WebCore::String, WTF::StrHash<WebCore::String>, WTF::HashTraits<WebCore::String>, WTF::HashTraits<WebCore::String> > const&) + 1197 (FrameLoaderMac.mm:172)
22  com.apple.WebCore              	0x0137a6b7 WebCore::FrameLoader::load(WebCore::FrameLoadRequest const&, bool, WebCore::Event*, WebCore::HTMLFormElement*, WTF::HashMap<WebCore::String, WebCore::String, WTF::StrHash<WebCore::String>, WTF::HashTraits<WebCore::String>, WTF::StrHash<WebCore::String> > const&) + 583 (FrameLoaderMac.mm:109)
23  com.apple.WebCore              	0x0139d35c WebCore::FrameLoader::urlSelected(WebCore::FrameLoadRequest const&, WebCore::Event*) + 190 (FrameLoader.cpp:2191)
24  com.apple.WebCore              	0x0139e2e2 WebCore::FrameLoader::urlSelected(WebCore::ResourceRequest const&, WebCore::String const&, WebCore::Event*, bool) + 654 (FrameLoader.cpp:362)
25  com.apple.WebCore              	0x012a17f0 WebCore::HTMLAnchorElement::defaultEventHandler(WebCore::Event*) + 2158 (HTMLAnchorElement.cpp:218)
26  com.apple.WebCore              	0x01226dc3 WebCore::EventTargetNode::dispatchGenericEvent(WTF::PassRefPtr<WebCore::Event>, int&, bool) + 1897 (EventTargetNode.cpp:260)
27  com.apple.WebCore              	0x012286a6 WebCore::EventTargetNode::dispatchEvent(WTF::PassRefPtr<WebCore::Event>, int&, bool) + 332 (EventTargetNode.cpp:296)
28  com.apple.WebCore              	0x0122753d WebCore::EventTargetNode::dispatchMouseEvent(WebCore::AtomicString const&, int, int, int, int, int, int, bool, bool, bool, bool, bool, WebCore::Node*, WTF::PassRefPtr<WebCore::Event>) + 691 (EventTargetNode.cpp:454)
29  com.apple.WebCore              	0x01227be8 WebCore::EventTargetNode::dispatchMouseEvent(WebCore::PlatformMouseEvent const&, WebCore::AtomicString const&, int, WebCore::Node*) + 398 (EventTargetNode.cpp:381)
30  com.apple.WebCore              	0x013bc724 WebCore::EventHandler::dispatchMouseEvent(WebCore::AtomicString const&, WebCore::Node*, bool, int, WebCore::PlatformMouseEvent const&, bool) + 572 (EventHandler.cpp:1050)
31  com.apple.WebCore              	0x013bcd48 WebCore::EventHandler::handleMouseReleaseEvent(WebCore::PlatformMouseEvent const&) + 622 (EventHandler.cpp:881)
32  com.apple.WebCore              	0x013b71b5 WebCore::EventHandler::mouseUp(NSEvent*) + 427 (EventHandlerMac.mm:873)
33  com.apple.WebKit               	0x003439e1 -[WebHTMLView mouseUp:] + 273 (WebHTMLView.m:3017)
34  com.apple.AppKit               	0x9335c42b -[NSWindow sendEvent:] + 5403
35  com.apple.Safari               	0x000230c6 0x1000 + 139462
36  com.apple.AppKit               	0x9334e350 -[NSApplication sendEvent:] + 5023
37  com.apple.Safari               	0x00022c56 0x1000 + 138326
38  com.apple.AppKit               	0x93278dfe -[NSApplication run] + 547
39  com.apple.AppKit               	0x9326cd2f NSApplicationMain + 573
40  com.apple.Safari               	0x0005f54a 0x1000 + 386378
41  com.apple.Safari               	0x0005f471 0x1000 + 386161

[David Kilzer (:ddkilzer)]

I tried adding a nil check in_webIsDataProtocolURL:(NSURL *), but this causes odd behavior if you continue to command-click on the new tab or the original tab.  There is a bug somewhere up the stack that is not creating a valid URL when fragments are involved.

[David Kilzer (:ddkilzer)]

In a release build (like WebKit nightly r18403), command-clicking the link on the test case (attachment 12006 [details]) causes the state of the first page to get stuck in loading mode.

[David Kilzer (:ddkilzer)]

*** Bug 12024 has been marked as a duplicate of this bug. ***

[Mark Rowe (bdash)]

Dave, is this still occurring? 

[David Kilzer (:ddkilzer)]

This no longer causes an assertion failure as of locally-built debug build of WebKit r18895 with Safari 2.0.4 (419.3) on Mac OS X 10.4.8 (8L127).  WebKit Nightly r18899 is fixed as well.

There are still two regressions, though.

Regression 1. Command-clicking the link of the test case page (Attachment 12006 [details]) now causes the title(!) to change (on both debug and release builds) from the URL to:

"unknown" failed to load

Steps to reproduce:

1. Open this bug.
2. Command click on Test case attachment.
3. Switch tabs to Test case page.
4. Note title of Test case page (the URL).
5. Command-click on link.
6. Notice how title changes (but should not).

Regression 2. After clicking (not command-clicking) on the Test case's link, further command-clicking does not open new tabs.

Steps to reproduce:

1. Open Test case.
2. Click on link (do not command-click).
3. Command-click on link to open new tab.
4. Note that nothing happens in Step 3 (no new tab is opened).

--

I need to check on behavior in shipping Safari, but I'm pretty sure the above two items are still regressions.

[David Kilzer (:ddkilzer)]

(In reply to comment #7)
> I need to check on behavior in shipping Safari, but I'm pretty sure the above
> two items are still regressions.

Okay, weird.

"Regression 1" above is a regression.  It does not happen in shipping Safari 2.0.4 (419.3) on Mac OS X 10.4.8 (8L127).

Additionally, the first command-click after opening the Test case opens a new window.  Any subsequent command-clicks in the same window simply register as normal clicks (taking you to the anchor URL).  In the newly opened tab, you may also only command-click once, after that all additional command-clicks on that link are registered as normal clicks.

"Regression 2" on shipping Safari does not open any tabs at all once a normal click is done to the anchor in the Test case page.  Not sure if ToT behavior is a progression or regression, though.

[David Kilzer (:ddkilzer)]

What would Firefox do?

Firefox obviously doesn't change the title of the page when command-clicking on the link in the Test case page.  (Regression #1 in Comment #7.)

Firefox also lets you open multiple tabs after clicking on the link once, then command-clicking multiple times.  (One tab is opened for each command-click.)  (Regression #2 in Comment #7.)

So I'd say WebKit's behavior still needs to be fixed in that respect.

[Mark Rowe (bdash)]

Can we split this into two bugs -- one for each of the regressions?

[David Kilzer (:ddkilzer)]

(In reply to comment #10)
> Can we split this into two bugs -- one for each of the regressions?

Yeah, I thought about doing that after I went to bed.  Actually, I think it'd be best to close this bug and open two new ones.  I'll do that shortly.

[David Kilzer (:ddkilzer)]

Marking this bug as RESOLVED/FIXED since the assertion failure no longer occurs.

Will open two new bugs for regressions in Comment #7.

[David Kilzer (:ddkilzer)]

(In reply to comment #10)
> Can we split this into two bugs -- one for each of the regressions?

Maybe you meant that I should open two new bugs.  :)

See Bug 12299 (title change) and Bug 12300 (command-clicking).