Bug 114645

Summary: Update layout test verifying Tab behaves correctly after linking to fragment ID
Product: WebKit Reporter: James Craig <jcraig>
Component: AccessibilityAssignee: Mario Sanchez Prada <mario>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, cfleizach, commit-queue, jcraig, mario, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch proposal
none
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion
none
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2
none
Patch proposal none

Description James Craig 2013-04-15 15:43:10 PDT
From bug 17450:

> The only suggestion I think I could make is whether it would make sense to add some bits to the test to check that pressing "Tab" after moving the focus by following a link works properly.

We'll do that here instead of recycling the reviewed patch.
Comment 1 Mario Sanchez Prada 2013-04-16 02:17:14 PDT
Created attachment 198279 [details]
Patch proposal

Thanks for filing this bug after the other one got fixed. Now proposing a patch for this one
Comment 2 Mario Sanchez Prada 2013-04-16 02:19:59 PDT
Adding Chris to ask for review
Comment 3 Build Bot 2013-04-16 06:32:04 PDT
Comment on attachment 198279 [details]
Patch proposal

Attachment 198279 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/165027

New failing tests:
fast/dom/fragment-activation-focuses-target.html
Comment 4 Build Bot 2013-04-16 06:32:05 PDT
Created attachment 198325 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.2
Comment 5 James Craig 2013-04-17 01:30:20 PDT
Reassigning to Mario to resolve the layout test failure.
Comment 6 Build Bot 2013-04-17 18:24:56 PDT
Comment on attachment 198279 [details]
Patch proposal

Attachment 198279 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/16133

New failing tests:
fast/dom/fragment-activation-focuses-target.html
Comment 7 Build Bot 2013-04-17 18:24:58 PDT
Created attachment 198666 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.2
Comment 8 Mario Sanchez Prada 2013-04-19 09:10:26 PDT
(In reply to comment #5)
> Reassigning to Mario to resolve the layout test failure.

According to the diff, the problem seems to be this:

Verify that the focus is on the link.
 PASS document.activeElement is link1
 Verify Tab behaves correctly before clicking the link.
-PASS document.activeElement is document.getElementById('link2')
-PASS document.activeElement is document.getElementById('link1')
+FAIL document.activeElement should be file:///Volumes/Data/EWS/WebKit/LayoutTests/fast/dom/fragment-activation-focuses-target.html#fragment2. Was [object HTMLDivElement].
+FAIL document.activeElement should be file:///Volumes/Data/EWS/WebKit/LayoutTests/fast/dom/fragment-activation-focuses-target.html#fragment1. Was [object HTMLBodyElement].
 Click the link and verify that focus has moved to the fragment.
 PASS document.activeElement is document.getElementById('fragment1')


... which I guess is perhaps happening due to some objects being focusable in the Mac but not in the Gtk port (where it works fine), or maybe it's because the tab is not where I was supposing it to be after setting the focus initially on link1 and before actually clicking the link, which I'm not sure whether it would be a bug or not in the context of Mac.

In any case, what seems to be working in this test, also in the mac, is that tab behaves as expected AFTER clicking the link, which is the most important thing to test here.

So, perhaps the right thing to do is just to leave those checks after clicking the link here and remove the other ones, and maybe file a new bug for that strange initial situation that only seemed to happen in Mac.

What do you think?
Comment 9 chris fleizach 2013-04-19 09:13:31 PDT
(In reply to comment #8)
> (In reply to comment #5)
> > Reassigning to Mario to resolve the layout test failure.
> 
> According to the diff, the problem seems to be this:
> 
> Verify that the focus is on the link.
>  PASS document.activeElement is link1
>  Verify Tab behaves correctly before clicking the link.
> -PASS document.activeElement is document.getElementById('link2')
> -PASS document.activeElement is document.getElementById('link1')
> +FAIL document.activeElement should be file:///Volumes/Data/EWS/WebKit/LayoutTests/fast/dom/fragment-activation-focuses-target.html#fragment2. Was [object HTMLDivElement].
> +FAIL document.activeElement should be file:///Volumes/Data/EWS/WebKit/LayoutTests/fast/dom/fragment-activation-focuses-target.html#fragment1. Was [object HTMLBodyElement].
>  Click the link and verify that focus has moved to the fragment.
>  PASS document.activeElement is document.getElementById('fragment1')
> 
> 
> ... which I guess is perhaps happening due to some objects being focusable in the Mac but not in the Gtk port (where it works fine), or maybe it's because the tab is not where I was supposing it to be after setting the focus initially on link1 and before actually clicking the link, which I'm not sure whether it would be a bug or not in the context of Mac.
> 
> In any case, what seems to be working in this test, also in the mac, is that tab behaves as expected AFTER clicking the link, which is the most important thing to test here.
> 
> So, perhaps the right thing to do is just to leave those checks after clicking the link here and remove the other ones, and maybe file a new bug for that strange initial situation that only seemed to happen in Mac.
> 
> What do you think?

That sounds fine to just update the after the click tabs. I've noticed the initial focus of the web page in layout tests is hard to nail down across platforms
Comment 10 Mario Sanchez Prada 2013-04-22 01:47:51 PDT
Created attachment 199003 [details]
Patch proposal

New patch checking the tab behavior just after following the link, as per Chris's comments
Comment 11 WebKit Commit Bot 2013-04-22 08:01:16 PDT
Comment on attachment 199003 [details]
Patch proposal

Clearing flags on attachment: 199003

Committed r148882: <http://trac.webkit.org/changeset/148882>
Comment 12 WebKit Commit Bot 2013-04-22 08:01:19 PDT
All reviewed patches have been landed.  Closing bug.