LayoutTests/fast/events/menu-key-context-menu-element-focus.html - Issue 766143002: Fix contextmenu event location for menu key in an iframe

Side by Side Diff: LayoutTests/fast/events/menu-key-context-menu-element-focus.html

Issue 766143002: Fix contextmenu event location for menu key in an iframe (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master

Patch Set: Changes as per review comments. Created 6 years ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
1 <html>	1 <html>

2 <head>	2 <head>

3 <title>	3 <title>

4 Test for contextmenu event target	4 Test for contextmenu event target

5 </title>	5 </title>

6 </head>	6 </head>

7 <body onload="test()">	7 <body onload="test()">

8 <p>This test checks if contextmenu event target is correct when triggered from m enu key.</p>	8 <p>This test checks if focus is on same element after dismissing context menu

	9 by esc key context menu came by menu key selection, so that when menu key is

	10 selected context menu will come on that element.</p>

9 <a href="" id="anchor" oncontextmenu="handleContextMenu(event);">Test</a>	11 <a href="" id="anchor" oncontextmenu="handleContextMenu(event);">Test</a>

10 <div id="result">FAIL</div>	12 <div id="result">FAIL</div>

11 <script>	13 <script>

12 var count = 0;	14 var count = 0;

	15 var anchorNode;

13 function handleContextMenu(e) {	16 function handleContextMenu(e) {

14 count++;	17 count++;

15 if(count === 2)	18 if(count === 2 && (e.currentTarget == anchorNode)) {
	Rick Byers 2014/12/10 03:09:24 Does this test actually fail without your patch? Does this test actually fail without your patch? currentTarget==anchorNode should always be true since the event handler was added on the anchorNode directly. I guess I don't understand why menu-key-context-menu.html isn't already covering everything you're testing here. What's fundamentally different about this test? From the code, I would have expected that you'd need to scroll the page in order to provoke the bug. What if you just change menu-key-context-menu.html to add some padding at the bottom of the document and call window.scrollTo(0, 30). That way the root view co-ordinate space and the contents co-ordinate space are different. If do that, does the test start failing without your patch (but still pass with your patch)? Deepak 2014/12/10 06:40:27 No , This test is not failing without my changes. Show quoted text On 2014/12/10 03:09:24, Rick Byers wrote: > Does this test actually fail without your patch? currentTarget==anchorNode > should always be true since the event handler was added on the anchorNode > directly. No , This test is not failing without my changes. Show quoted text > I guess I don't understand why menu-key-context-menu.html isn't already covering > everything you're testing here. What's fundamentally different about this test? Actually when you load http://jsfiddle.net/kvckb90w/2/show/ * Use tab key to bring focus on one of the buttons * Hit the context menu key Then context menu does not come on the button but off the button and on selecting 'esc' key, Button will lose focus and then again selecting 'Context menu' key context menu will come at the top left corner and context menu items are also different. But I am surprised that in the test case doing document.activeElement after hitting 'esc' giving still 'anchor' element as focused element. Show quoted text > From the code, I would have expected that you'd need to scroll the page in order > to provoke the bug. What if you just change menu-key-context-menu.html to add > some padding at the bottom of the document and call window.scrollTo(0, 30). > That way the root view co-ordinate space and the contents co-ordinate space are > different. If do that, does the test start failing without your patch (but > still pass with your patch)? I tried this, But still the test case is getting passed, as 'Anchor' element is not losing focus after scroll. I have added a padding in para as: <style> p.ex1 { padding: 100px 100px; } </style> <a href="" id="anchor" oncontextmenu="handleContextMenu(event);">Test</a> <div id="result">FAIL</div> <p class="ex1">Padding.</p> and call as: anchor.focus(); eventSender.keyDown("menu"); eventSender.keyDown("escape"); window.scroll(0,200); eventSender.keyDown("menu"); but as 'Anchor' element doesn't lose focus in test case, so it is getting passed even without my change. But in the issue case 'Button' is losing focus, as context menu is not on the button.
16 document.getElementById("result").innerHTML = "PASS";	19 document.getElementById("result").innerHTML = "PASS";

	20 }

17 return false;	21 return false;

18 }	22 }

19

20 function test() {	23 function test() {

21 if (window.testRunner)	24 if (window.testRunner)

22 testRunner.dumpAsText();	25 testRunner.dumpAsText();

23	26

24 if (!window.eventSender)	27 if (!window.eventSender)

25 return;	28 return;

26	29

27 var anchor = document.getElementById("anchor");	30 anchorNode = document.getElementById("anchor");

28	31 anchor.focus();

29 var r = anchor.getBoundingClientRect();	32 eventSender.keyDown("menu");

30 var x = (r.left + r.right) / 2

31 var y = (r.top + r.bottom) / 2;

32 eventSender.mouseMoveTo(x, y);

33 eventSender.contextClick();

34 // Esc key to hide context menu	33 // Esc key to hide context menu

35 eventSender.keyDown("escape");	34 eventSender.keyDown("escape");

36 anchor.focus();

37 eventSender.keyDown("menu");	35 eventSender.keyDown("menu");

38

39 }	36 }

40 </script>	37 </script>

41 </body>	38 </body>

42 </html>	39 </html>

OLD	NEW

« no previous file with comments | « LayoutTests/NeverFixTests ('k') | LayoutTests/fast/events/menu-key-context-menu-element-focus-expected.txt » ('j') | Source/core/page/EventHandler.cpp » ('J')