Bug 138457 - Canvas with focusable fallback content does not scroll into view when focus is moved to fallback
Summary: Canvas with focusable fallback content does not scroll into view when focus i...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joanmarie Diggs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-11-06 00:17 PST by Joanmarie Diggs
Modified: 2016-01-03 07:24 PST (History)
6 users (show)

See Also:


Attachments
test case (403 bytes, text/html)
2014-11-06 00:17 PST, Joanmarie Diggs
no flags Details
Patch (4.19 KB, patch)
2014-11-06 00:24 PST, Joanmarie Diggs
no flags Details | Formatted Diff | Diff
Patch (9.18 KB, patch)
2015-02-18 02:21 PST, Joanmarie Diggs
mcatanzaro: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joanmarie Diggs 2014-11-06 00:17:53 PST
Created attachment 241091 [details]
test case

Steps to reproduce:
1. Load the attached test case in Safari or Epiphany
2. Press Tab to move focus to Link 1
3. Press Tab again to move focus to Link 2 (which is fallback content and not rendered)

Expected results: The parent canvas element would be scrolled into view.

Actual results: Link 1 loses the focus ring, but the canvas element is not scrolled into view making it difficult to identify what has focus.
Comment 1 Joanmarie Diggs 2014-11-06 00:24:28 PST
Created attachment 241092 [details]
Patch
Comment 2 Joanmarie Diggs 2015-02-18 02:21:21 PST
Created attachment 246810 [details]
Patch
Comment 3 Michael Catanzaro 2016-01-02 10:36:15 PST
Comment on attachment 246810 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=246810&action=review

> Source/WebCore/dom/Element.cpp:2053
> +    else if (auto* canvas = ancestorsOfType<HTMLCanvasElement>(*this).first()) {

Make sure canvas is guaranteed to be non-null.
Comment 4 Joanmarie Diggs 2016-01-03 07:24:38 PST
Thanks for the review. I had forgotten about this patch.

That said, the previous/obsoleted patch (or some revised version thereof) may need to be the approach taken. The spec changed and scrolling to the top is no longer a requirement. (This is a good thing because scrolling to the top is inconsistent with what some user agents do when other focusable elements need to be scrolled on screen.) See http://www.w3.org/TR/2dcontext/#drawing-paths-to-the-canvas.