Bug 50730

Summary: Spatial Navigation: Crash when handling iframe of size 0.
Product: WebKit Reporter: Yael <yael>
Component: AccessibilityAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ademar, commit-queue, tonikitoo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 46905    
Attachments:
Description Flags
Patch. none

Yael
Reported 2010-12-08 19:32:19 PST
While adding more new tests I noticed that if we have a frame of size 0, we would get into infinite loop and eventually crash. The reason is that when the algorithm sees a starting rect of size 0, it assumes that there is no focused node, thus restarts itself.
Attachments
Patch. (4.89 KB, patch)
2010-12-08 19:39 PST, Yael
no flags
Yael
Comment 1 2010-12-08 19:39:33 PST
Antonio Gomes
Comment 2 2010-12-09 07:33:44 PST
Comment on attachment 76005 [details] Patch. View in context: https://bugs.webkit.org/attachment.cgi?id=76005&action=review > WebCore/page/FocusController.cpp:422 > // Ignore iframes that don't have a src attribute Please also update comment.
WebKit Commit Bot
Comment 3 2010-12-09 08:26:13 PST
Comment on attachment 76005 [details] Patch. Rejecting patch 76005 from commit-queue. Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sl', 'build', '--no-clean', '--no-update', '--build-style=both']" exit_code: 1 ERROR: Working directory has local commits, pass --force-clean to continue. Full output: http://queues.webkit.org/results/6948010
WebKit Commit Bot
Comment 4 2010-12-09 11:37:00 PST
Comment on attachment 76005 [details] Patch. Clearing flags on attachment: 76005 Committed r73634: <http://trac.webkit.org/changeset/73634>
WebKit Commit Bot
Comment 5 2010-12-09 11:37:06 PST
All reviewed patches have been landed. Closing bug.
Ademar Reis
Comment 6 2010-12-13 11:14:30 PST
I can't just cherry-pick it to qtwebkit-2.1 because of changes from bug 47142. I'll fix the conflict as below. Yael/Antonio, could you please confirm it's OK? - if (candidate.node->isFrameOwnerElement() && !static_cast<HTMLFrameOwnerElement*>(candidate.node)->contentFrame()) + if (candidate.node->isFrameOwnerElement() && (!static_cast<HTMLFrameOwnerElement*>(candidate.node)->contentFrame() || candidate.rect.isEmpty()))
Yael
Comment 7 2010-12-13 12:01:14 PST
(In reply to comment #6) > I can't just cherry-pick it to qtwebkit-2.1 because of changes from bug 47142. > > I'll fix the conflict as below. Yael/Antonio, could you please confirm it's OK? > > - if (candidate.node->isFrameOwnerElement() && !static_cast<HTMLFrameOwnerElement*>(candidate.node)->contentFrame()) > + if (candidate.node->isFrameOwnerElement() && (!static_cast<HTMLFrameOwnerElement*>(candidate.node)->contentFrame() || candidate.rect.isEmpty())) Looks good to me. thanks!
Ademar Reis
Comment 8 2010-12-13 12:24:53 PST
Revision r73634 cherry-picked into qtwebkit-2.1 with commit 7bebc59 <http://gitorious.org/webkit/qtwebkit/commit/7bebc59>
Note You need to log in before you can comment on or make changes to this bug.