Opened 11 years ago
Closed 11 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 )
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 11 years ago by
comment:2 Changed 11 years ago by
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 11 years ago by
should have said "Left click in the textbox containing the text top-right and the" ...
comment:4 Changed 11 years ago by
(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 11 years ago by
| Description: | modified (diff) |
|---|---|
| Owner: | set to bill |
| Status: | new → assigned |
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:
- 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>).
- Refocusing on the <body>/<td> causes the screen to scroll to the top of the <body>/<td>
- 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 11 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
(In [19927]) Fix problems where closing a menu would make the screen's scroll jump to the top. Fixes two underlying IE issues:
- 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.
- 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 11 years ago by
(In [19933]) Refs #9735 - change back where it breaks menus in DropDownSelect? and context menus for now...
comment:8 Changed 11 years ago by
comment:9 Changed 11 years ago by
comment:10 Changed 11 years ago by
comment:11 Changed 11 years ago by
comment:12 Changed 11 years ago by
(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 11 years ago by
| Resolution: | fixed |
|---|---|
| Status: | closed → reopened |
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 11 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | reopened → closed |
(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.
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.