Opened 10 years ago

Closed 10 years ago

#9735 closed defect (fixed)

Menu: refocus broken on IE

Reported by: bill Owned by: bill
Priority: high Milestone: 1.4
Component: Dijit Version: 1.3.2
Keywords: Cc: Jared Jurkiewicz
Blocked By: Blocking:

Description (last modified by bill)

See test_Menu.html. Scroll down the page and then right click to get the context menu (over a blank area of the page). Select "Not checked" or any other choice from the context menu.

The page scroll jumps back to near the top.

Works correctly on FF3.5/mac but fails on IE6, IE7, and IE8. The plain FocusManager test works OK on IE though.

This is causing the problem with TabContainer scrolling to the top of the page.

Change History (14)

comment:1 Changed 10 years ago by bill

When the user right clicks over either <body> or a large node (like the <td> in test_Menu.html that takes up 90% of the screen), when the menu closes it tries to refocus the <body>/<td> and that causes thepage to scroll up.

I thought it was calling dijit.scrollIntoView() but it doesn't seem to be, it just seems to be calling node.focus(). Hmm.

comment:2 Changed 10 years ago by Douglas Hays

I cannot recreate this problem - I tried IE6 and 8. I did see another related and ugly problem though on both browsers:
Left click in the textbox and the context menu appears.
Press the ESC key and EVERYTHING on the page is selected and the page is scrolled to the bottom.

comment:3 Changed 10 years ago by Douglas Hays

should have said "Left click in the textbox containing the text top-right and the" ...

comment:4 Changed 10 years ago by bill

(In [19923]) TabContainer? navigation menu shouldn't try to refocus to previously focused node, since focus will be moved to a TabButton?. Removing the refocus avoids the screen scroll-jump problem on IE. Refs #9004, #9735 !strict.

comment:5 Changed 10 years ago by bill

Description: modified (diff)
Owner: set to bill
Status: newassigned

My description of how to reproduce the error was unclear, talked to Doug and explained it better and then he could reproduce it... updating description now.

There are three issues causing the problem:

  1. Clicking on the blank area of the screen on IE registers the current focus (dijit._curFocus) as as the <body>, or in the case of test_Menu.html, a <td> taking up 90% of the document, even though neither of those blocks is focusable. This is how the onactivate event works in IE. It would be better if that was just registered as a blur of the previously focused <input> etc. (if there was a previously focused <input>).
  2. Refocusing on the <body>/<td> causes the screen to scroll to the top of the <body>/<td>
  3. Also, restoring a bookmark to the <body>/<td>, even a collapsed bookmark, also causes the scroll. That's because on IE the bookmark contains more than just a selection.

comment:6 Changed 10 years ago by bill

Resolution: fixed
Status: assignedclosed

(In [19927]) Fix problems where closing a menu would make the screen's scroll jump to the top. Fixes two underlying IE issues:

  1. Clicking on the blank area of the screen on IE was registering the dijit._curFocus as the <body>, or in the case of test_Menu.html, a <td> taking up 90% of the document, even though neither of those blocks was focusable. Changed the code to ignore onactivate events on such nodes.
  2. Restoring a bookmark to the <body>/<td>, even a collapsed bookmark, was causing the scroll. That's because on IE the bookmark contains more than just a selection. Changed the code to no-op on IE when the bookmark is collapsed. (This is a little overboard; it means that caret position inside of a <textarea> will also not be restored, might want to check in a more refined solution later.)

Fixes #9735 !strict.

comment:7 Changed 10 years ago by Nathan Toone

(In [19933]) Refs #9735 - change back where it breaks menus in DropDownSelect? and context menus for now...

comment:8 Changed 10 years ago by bill

(In [19948]) Refine cases where it ignores focus events; should treat focus on menu items etc. as real focus events, but not focus on containing blocks like <td> holding most of the page content in test_Menu.html.

Refs #9735 !strict.

comment:9 Changed 10 years ago by bill

(In [19955]) Make isTabNavigable() return true for editor iframes. It's pretty messy detecting which iframes are editors, wish there was a better way. Fixes #9766, refs #9735 !strict.

comment:10 Changed 10 years ago by liucougar

(In [19965]) refs #9766: correctly handle contentEditable elements (they should be considered as focusable) also refs #9735 also fixed some strict warnings

comment:11 Changed 10 years ago by bill

(In [19966]) Try again to get focus on IE working properly. Clicking a non-focusable node (including <body>) shouldn't be considered a focus, but it should at least be considered a node touch.

Refs #9735, fixes #9782 !strict.

comment:12 Changed 10 years ago by bill

(In [20014]) Make alphanumeric navigation tests in Tree_a11y.html on IE work again.

Tree is unusual in that it puts focus on a node with tabIndex == -1, and then later sets that node's tabIndex = 0. That confuses the focus.js code which partially ignores onactivate events on tabIndex == -1 nodes.

Not sure if Tree or the focus code is to blame but for now changing Tree to explicitly call dijit.focus().

Refs #9735 !strict.

comment:13 Changed 10 years ago by bill

Resolution: fixed
Status: closedreopened

Turns out this change breaks Editor._restoreSelection(), so I need to do it a different way.

When a menu closes it tries to restore focus and selection, which of course means that as the menu open it tries to save the focus and selection.

getFocus() has some fancy code to use the previously focused node rather than the currently focused node. The reason for that fancy code is so that if a user is in a <textarea> etc, and then clicks a button etc. (blurring the textarea and perhaps focusing the button), the button's onclick handler is able to do something with the previously focused node and selection.

However, that bookmarking code (which BTW AFAIK isn't being used by anyone) doesn't work in this case in this ticket. When right clicking on a blank area of the screen (to show the menu) it correctly saves that (previous) focus is the <input>, but it also saves that current bookmark/selection as a reference to <body>, rather than whatever was selected in the <input>.

So, in that case it's better to just not save a bookmark at all.

comment:14 Changed 10 years ago by bill

Resolution: fixed
Status: reopenedclosed

(In [20220]) The editor depends on moveToBookmark() to restore caret position, so moveToBookmark() can't just do nothing for a collapsed selection.

As for getFocus(), when it returns the previous focus rather than the current focus, it can't correctly report the selection in the previously focused node, so changing it to just return nothing about the selection.

Fixes #9938, #9735 !strict

Note: See TracTickets for help on using tickets.