Bug 8823 - SVG does not support tabbing navigation between web page links
Summary: SVG does not support tabbing navigation between web page links
Status: RESOLVED WORKSFORME
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Nobody
URL: http://www.peepo.co.uk
Keywords: HasReduction, Regression
: 12631 (view as bug list)
Depends on: 14027 10849
Blocks:
  Show dependency treegraph
 
Reported: 2006-05-10 00:53 PDT by jay
Modified: 2010-04-27 12:14 PDT (History)
6 users (show)

See Also:


Attachments
A partial patch (2.56 KB, patch)
2006-09-12 23:11 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
reduced testcase (1.19 KB, image/svg+xml)
2007-03-12 10:34 PDT, jay
no flags Details
Updated (complete) patch (4.09 KB, patch)
2007-06-06 13:41 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
automated layout test (hits ASSERT in WebView) (653 bytes, image/svg+xml)
2007-06-06 14:17 PDT, Eric Seidel (no email)
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description jay 2006-05-10 00:53:45 PDT
keyboard support for SVG withing webkit Safari is currently broken

Reproducible: Always

Steps to Reproduce:
1.open URI 
2.try to navigate through the homepage using the keyboard 'tab' or other key
3.

Actual Results:  
tab key onkeydown not recognised

Expected Results:  
focus should change to next anchor

It should be possible to cycle through the links on any SVG page as one can with HTML using a keyboard
Comment 1 Eric Seidel (no email) 2006-05-10 10:37:09 PDT
Yeah, this just hasn't been wired up yet.  Definitely a bug.
Comment 2 jay 2006-06-30 13:38:27 PDT
although http://www.peepo.co.uk tabbing works in Mozilla, this is itself a bug :-)

please use http://www.peepo.co.uk/index-opera.svg.

simply the second tabs to anchors, whereas mozilla uses groups.
Erik@Opera kindly pointed suggested anchors was correct.

Comment 3 Eric Seidel (no email) 2006-09-12 23:11:13 PDT
Created attachment 10522 [details]
A partial patch

This patch gets us part of the way there.  It doesn't seem to work quite right yet though.  I have not debugged why.  Even once we make SVGAElement's focusable, we'll have to find a way to draw the css outline property, as that's how WebKit does the link focusing.
Comment 4 jay 2007-03-12 10:34:10 PDT
Created attachment 13594 [details]
reduced testcase
Comment 5 jay 2007-03-20 09:54:08 PDT
olliej & harrison,
apologies for copying you in on this bug, it's SVG & Accessibility.

I understand you're all tied up on P1s.

keyboard accessibility is essential.
Comment 6 Mark Rowe (bdash) 2007-03-20 09:56:30 PDT
I don't think this needs to be a P1.
Comment 7 jay 2007-03-20 10:04:52 PDT
#6

bdash,

evidently you're not blind,
and also in control of a mouse.

http://www.w3.org/TR/SVG-access/#events
http://www.w3.org/TR/WAI-WEBCONTENT-TECHS/#def-device-independent

Comment 8 Mark Rowe (bdash) 2007-03-20 10:12:41 PDT
Actually I lack a mouse.  That's besides the point though.

The normal criteria for determining whether something qualifies as a P1 is if it is a crash, security problem, regression, obvious rendering issue, or a bug that affect major sites.
Comment 9 jay 2007-03-20 10:14:06 PDT
The attachment works in Opera, Camino and Mozilla
Comment 10 Eric Seidel (no email) 2007-06-06 13:41:34 PDT
Created attachment 14889 [details]
Updated (complete) patch
Comment 11 Eric Seidel (no email) 2007-06-06 14:17:42 PDT
Created attachment 14891 [details]
automated layout test (hits ASSERT in WebView)
Comment 12 Eric Seidel (no email) 2007-06-06 14:18:35 PDT
This is the backtrace for the ASSERT which is tripped by this layout test:

#0	0x0026426c in -[WebView becomeFirstResponder] at WebView.mm:2251
#1	0x932cc483 in -[NSWindow makeFirstResponder:]
#2	0x93380fef in -[NSWindow selectKeyViewFollowingView:]
#3	0x002951d8 in WebChromeClient::takeFocus at WebChromeClient.mm:119
#4	0x013efedb in WebCore::Chrome::takeFocus at Chrome.cpp:85
#5	0x0143227f in WebCore::FocusController::advanceFocus at FocusController.cpp:155
#6	0x0143253c in WebCore::FocusController::advanceFocus at FocusController.cpp:112
#7	0x002477d2 in -[WebHTMLView becomeFirstResponder] at WebHTMLView.mm:3142
#8	0x932cc483 in -[NSWindow makeFirstResponder:]
#9	0x9344fcd3 in -[NSClipView becomeFirstResponder]
#10	0x932cc483 in -[NSWindow makeFirstResponder:]
#11	0x9344fc34 in -[NSScrollView becomeFirstResponder]
#12	0x932cc483 in -[NSWindow makeFirstResponder:]
#13	0x0025a8f3 in -[WebFrameView becomeFirstResponder] at WebFrameView.mm:391
#14	0x932cc483 in -[NSWindow makeFirstResponder:]
#15	0x002643a8 in -[WebView becomeFirstResponder] at WebView.mm:2275
#16	0x932cc483 in -[NSWindow makeFirstResponder:]
#17	0x93380fef in -[NSWindow selectKeyViewFollowingView:]
#18	0x002951d8 in WebChromeClient::takeFocus at WebChromeClient.mm:119
#19	0x013efedb in WebCore::Chrome::takeFocus at Chrome.cpp:85
#20	0x0143227f in WebCore::FocusController::advanceFocus at FocusController.cpp:155
#21	0x0143253c in WebCore::FocusController::advanceFocus at FocusController.cpp:112
#22	0x014083e1 in WebCore::EventHandler::defaultTabEventHandler at EventHandler.cpp:1681
#23	0x01409463 in WebCore::EventHandler::defaultKeyboardEventHandler at EventHandler.cpp:1419
#24	0x01239c99 in WebCore::EventTargetNode::defaultEventHandler at EventTargetNode.cpp:583
#25	0x01237fc4 in WebCore::EventTargetNode::dispatchGenericEvent at EventTargetNode.cpp:267
#26	0x01239909 in WebCore::EventTargetNode::dispatchEvent at EventTargetNode.cpp:308
#27	0x0106041d in WebCore::SVGElement::dispatchEvent at SVGElement.cpp:237
#28	0x01409350 in WebCore::EventHandler::defaultKeyboardEventHandler at EventHandler.cpp:1409
#29	0x01239c99 in WebCore::EventTargetNode::defaultEventHandler at EventTargetNode.cpp:583
#30	0x01237fc4 in WebCore::EventTargetNode::dispatchGenericEvent at EventTargetNode.cpp:267
#31	0x01239909 in WebCore::EventTargetNode::dispatchEvent at EventTargetNode.cpp:308
#32	0x0106041d in WebCore::SVGElement::dispatchEvent at SVGElement.cpp:237
#33	0x01238f3a in WebCore::EventTargetNode::dispatchKeyEvent at EventTargetNode.cpp:370
#34	0x014081a4 in WebCore::EventHandler::keyEvent at EventHandler.cpp:1375
#35	0x01405265 in WebCore::EventHandler::keyEvent at EventHandlerMac.mm:138
#36	0x0023f128 in -[WebHTMLView keyDown:] at WebHTMLView.mm:3419
#37	0x00004dad in -[EventSendingController keyDown:withModifiers:] at EventSendingController.m:367
#38	0x90a5ac56 in objc_msgSendv
#39	0x927f53b2 in -[NSInvocation invoke]
#40	0x003e1486 in KJS::Bindings::ObjcInstance::invokeMethod at objc_instance.mm:187
#41	0x003dcfd5 in KJS::RuntimeMethod::callAsFunction at runtime_method.cpp:89
#42	0x0041e24e in KJS::JSObject::call at object.cpp:98
#43	0x0044773d in KJS::FunctionCallDotNode::evaluate at nodes.cpp:790
#44	0x00444e93 in KJS::ExprStatementNode::execute at nodes.cpp:1723
#45	0x00444d9d in KJS::IfNode::execute at nodes.cpp:1742
#46	0x00442176 in KJS::SourceElementsNode::execute at nodes.cpp:2528
#47	0x0041adb4 in KJS::BlockNode::execute at nodes.cpp:1699
#48	0x0043f301 in KJS::Interpreter::evaluate at interpreter.cpp:365
#49	0x0127901f in WebCore::KJSProxy::evaluate at kjs_proxy.cpp:78
#50	0x013df4bb in WebCore::FrameLoader::executeScript at FrameLoader.cpp:712
#51	0x0102cd6f in WebCore::XMLTokenizer::endElementNs at XMLTokenizer.cpp:753
#52	0x0102ce07 in endElementNsHandler at XMLTokenizer.cpp:985
#53	0x91bff515 in xmlParseNotationDecl
#54	0x91be4d86 in xmlParseChunk
#55	0x0102be90 in WebCore::XMLTokenizer::write at XMLTokenizer.cpp:566
#56	0x013d4075 in WebCore::FrameLoader::write at FrameLoader.cpp:927
#57	0x013d41a7 in WebCore::FrameLoader::addData at FrameLoader.cpp:1583
#58	0x01102a01 in -[WebCoreFrameBridge addData:] at WebCoreFrameBridge.mm:288
#59	0x01105b9c in -[WebCoreFrameBridge receivedData:textEncodingName:] at WebCoreFrameBridge.mm:1427
#60	0x00233245 in -[WebHTMLRepresentation receivedData:withDataSource:] at WebHTMLRepresentation.mm:173
#61	0x0022e73b in -[WebDataSource(WebInternal) _receivedData:] at WebDataSource.mm:176
#62	0x002917c5 in WebFrameLoaderClient::committedLoad at WebFrameLoaderClient.mm:658
#63	0x013d0f6b in WebCore::FrameLoader::committedLoad at FrameLoader.cpp:3027
#64	0x013e1acd in WebCore::DocumentLoader::commitLoad at DocumentLoader.cpp:347
#65	0x013e1b26 in WebCore::DocumentLoader::receivedData at DocumentLoader.cpp:359
#66	0x013d0465 in WebCore::FrameLoader::receivedData at FrameLoader.cpp:2039
#67	0x013e33f0 in WebCore::MainResourceLoader::addData at MainResourceLoader.cpp:133
#68	0x013e5599 in WebCore::ResourceLoader::didReceiveData at ResourceLoader.cpp:208
#69	0x013e3725 in WebCore::MainResourceLoader::didReceiveData at MainResourceLoader.cpp:289
#70	0x013e51a0 in WebCore::ResourceLoader::didReceiveData at ResourceLoader.cpp:330
#71	0x013c0100 in -[WebCoreResourceHandleAsDelegate connection:didReceiveData:lengthReceived:] at ResourceHandleMac.mm:351
#72	0x92854afa in -[NSURLConnection(NSURLConnectionInternal) _sendDidReceiveDataCallback]
#73	0x92852ddb in -[NSURLConnection(NSURLConnectionInternal) _sendCallbacks]
#74	0x92852ab5 in _sendCallbacks
#75	0x9082bf92 in CFRunLoopRunSpecific
#76	0x9082bace in CFRunLoopRunInMode
#77	0x92823d3a in -[NSRunLoop runMode:beforeDate:]
#78	0x0000b09d in runTest at DumpRenderTree.m:1458
#79	0x00006f7e in dumpRenderTree at DumpRenderTree.m:526
#80	0x000071e6 in main at DumpRenderTree.m:572
Comment 13 Eric Seidel (no email) 2007-06-06 14:22:52 PDT
I'm betting that the implementation of
static Node* deepFocusableNode(FocusDirection direction, Node* node, KeyboardEvent* event)

in FocusController is our problem.  It seems to be looking for a HTMLFrameOwnerElement, which I doubt an SVG document has.
Comment 14 Eric Seidel (no email) 2007-06-06 15:01:35 PDT
If I make it send option-\t instead, it doesn't hit the ASSERT (but it also fails the test).  option-\t is required in my Safari browser, I guess because I don't have some preference turned on to allow link-tabbing.  I'm not sure which preference is on in DRT.

I believe it's hitting the ASSERT because it's not able to find any node which responds yes to isKeyboardFocusable (which makes sense, because they all respond "NO" when keyboard tabbing it turned off).  It doesn't seem that the focus code is prepared for this eventuality however.
Comment 15 Eric Seidel (no email) 2007-06-07 10:39:53 PDT
*** Bug 12631 has been marked as a duplicate of this bug. ***
Comment 16 Eric Seidel (no email) 2007-06-07 10:44:02 PDT
Ha!  Turns out that this is not an SVG bug after all.  This happens with xhtml documents too!  I've filed bug 14027 to track the ASSERT/crash.
Comment 17 Eric Seidel (no email) 2007-06-07 11:22:13 PDT
To clarify:  The patch still stands for review.  It can be landed.  We just can't land the layout test until bug 14027 is fixed.
Comment 18 Oliver Hunt 2007-06-08 02:26:19 PDT
Comment on attachment 14889 [details]
Updated (complete) patch

r=me
Comment 19 Eric Seidel (no email) 2007-06-08 02:29:56 PDT
Landed on feature branch as r22066.
Comment 20 jay 2007-09-15 01:33:51 PDT
I don't seem to have received an email regarding this having been "fixed"
however, using Safari 3.0.3 webkit r25571 seems totally broken.
Comment 21 Oliver Hunt 2007-09-15 02:11:15 PDT
This has been fixed in the feature branch (r22066)
Comment 22 jay 2007-09-15 02:30:08 PDT
#21  how about a link? 
when will the 'feature' branch enter the trunk?

the comments so far fall short of being helpful to the bug reporter

cheers
Comment 23 Oliver Hunt 2007-09-15 02:37:42 PDT
The bug is fixed, if you checkout the feature branch you can verify that it is fixed.  There is no date we can provide for when the feature branch will be merged into trunk, nor when a subsequent release will occur.

All that matters is that the bug is fixed, and will eventually end up in a release.
Comment 24 jay 2007-09-15 05:47:13 PDT
#23 

Oliver wrote "checkout the feature branch"

however this isn't a link, let alone a method.
a search with google provided no further information.

is there a build, or is this cvs only?
where is it?

as the bug reporter, it's hard for me to agree this is 'fixed' without further information.

also iirc it is quite usual for developers to consider a bug fixed, when in fact the reporter is not of the same opinion....
Comment 25 Mark Rowe (bdash) 2007-09-15 06:10:57 PDT
Jay, you can check out and build the feature branch in a similar manner to the normal (eg, trunk) build process.  Follow the instructions starting from <http://webkit.org/building/checkout.html>, but replace the step that says:

svn checkout http://svn.webkit.org/repository/webkit/trunk WebKit

with:

svn checkout http://svn.webkit.org/repository/webkit/branches/feature-branch WebKit

Then if you follow the build steps through to completion, you will have a built copy of WebKit from the feature branch which contains the fix for this issue.
Comment 26 jay 2007-09-15 13:50:28 PDT
#25

cheers Mark, well all went okay till build, ran for a short while and then:

....
CompileC /Users/Jay/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/ppc/WebCoreAXObject.o /Users/Jay/WebKit/WebCore/bridge/mac/WebCoreAXObject.mm normal ppc objective-c++ com.apple.compilers.gcc.4_0
    cd /Users/Jay/WebKit/WebCore
    /usr/bin/gcc-4.0 -x objective-c++ -arch ppc -pipe -Wno-trigraphs -fno-exceptions -fno-rtti -fobjc-exceptions -fpascal-strings -fasm-blocks -O2 -Werror -Wnon-virtual-dtor -Wnewline-eof -DNDEBUG -DENABLE_SVG -DENABLE_SVG_EXPERIMENTAL_FEATURES -DENABLE_XPATH -DENABLE_XSLT -DENABLE_VIDEO -fmessage-length=0 -fobjc-direct-dispatch -fobjc-gc -mtune=G5 -fvisibility-inlines-hidden -fno-threadsafe-statics -mmacosx-version-min=10.4 -gdwarf-2 -I/Users/Jay/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/WebCore.hmap -Wall -W -Wcast-align -Wchar-subscripts -Wformat-security -Wmissing-format-attribute -Wpointer-arith -Wwrite-strings -Wno-format-y2k -Wno-unused-parameter -Wno-long-double -Wundef -Wshorten-64-to-32 -F/Users/Jay/WebKit/WebKitBuild/Release -I/Users/Jay/WebKit/WebKitBuild/Release/include -IForwardingHeaders -Iicu -I/usr/include/libxslt -I/usr/include/libxml2 -I/Users/Jay/WebKit/WebKitBuild/Release/DerivedSources/WebCore -I/Users/Jay/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/DerivedSources -include /Library/Caches/com.apple.Xcode.501/SharedPrecompiledHeaders/WebCorePrefix-emjzgmqqjsmxpbcgzxwibfbkfkom/WebCorePrefix.h -c /Users/Jay/WebKit/WebCore/bridge/mac/WebCoreAXObject.mm -o /Users/Jay/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/ppc/WebCoreAXObject.o
/Users/Jay/WebKit/WebCore/bridge/mac/WebCoreAXObject.mm: In function 'BOOL -[WebCoreAXObject accessibilityIsIgnored](WebCoreAXObject*, objc_selector*)':
/Users/Jay/WebKit/WebCore/bridge/mac/WebCoreAXObject.mm:820: internal compiler error: Bus error
Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://developer.apple.com/bugreporter> for instructions.
{standard input}:11105:FATAL:.abort  detected.  Assembly stopping.
** BUILD FAILED **
jay:~ Jay$ 
Comment 27 jay 2009-04-17 04:47:34 PDT
I had thought the 'feature' branch was discontinued.

what is the delay? any clues please?

regards
Comment 28 Eric Seidel (no email) 2009-04-17 07:53:28 PDT
It would appear this regressed.
Comment 29 Eric Seidel (no email) 2009-04-29 13:06:27 PDT
Comment on attachment 14889 [details]
Updated (complete) patch

Clearing review bit so it's out of the review queue.
Comment 30 jay 2009-04-29 14:15:48 PDT
#29 eric could you clarify, please.

does this comment suggest that your patch doesn't need reviewing?
or perhaps that this bug doesn't need reviewing for the next 'landing'?

or something else?

cheers
Comment 31 Eric Seidel (no email) 2009-04-29 14:19:06 PDT
(In reply to comment #30)
> #29 eric could you clarify, please.

The code is already in the tree.  We never landed (aka committed) the layout test because of bug 14027.  Our behavior regressed (which could have been avoided had we landed the layout test as well).

Eventually someone will get around to fixing this again.
Comment 32 Eric Seidel (no email) 2009-05-03 15:38:44 PDT
turns out our navigation is just fine.  It's just that our repaint rects do not include the outline (thus it's never painted).
Comment 33 jay 2010-04-27 11:04:31 PDT
eric,

what's the status on this bug?