1498816 - PageDown/PageUp in facebook chat loses focus

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect, P2)

Description

[Andrei Ancuta]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:63.0) Gecko/20100101 Firefox/63.0

Steps to reproduce:

Open facebook, log in, open a chat box with someone
Make sure the chat box is focused i.e. you can type a message in it, the chat box bar is blue
Press fn+arrowDown (this is pgdn for me)


Actual results:

Facebook page scrolls down
The chat box loses focus


Expected results:

Facebook page scrolls down
The chat box doesn't lose focus -- I should be able to continue to type in the chat box without having to focus it again.


Please note that this doesn't happen when scrolling with the mouse or trackpad

Comment 1

[Andrei Ancuta]

Does not reproduce in Chrome.
Does reproduce in Safe Mode.

Comment 2

[Alice0775 White]

I can reproduce the issue on Nihjtly64.0a1 Windows10, but not on Chrome.

Comment 3

[laszlo.bialis]

Hi,

Managed to reproduce the issue on the latest Nightly 64.0a1 Build ID: 20181016220050 on Windows 10 and 7 x64 and Ubuntu 16.04 x86, however the issue is not reproducible on MacOS 10.14.

I am assigning a component to this issue in order to involve the development team and get an opinion on this.

Comment 4

[Andrew Overholt [:overholt]]

Masayuki might have an idea of what's going on here. If not, please needinfo smaug.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I got stack when blur event handler is called with PageDown/PageUp.

#01: mozilla::EventListenerManager::HandleEventSubType (c:\mozilla\src\dom\events\EventListenerManager.cpp:1107)
#02: mozilla::EventListenerManager::HandleEventInternal (c:\mozilla\src\dom\events\EventListenerManager.cpp:1311)
#03: mozilla::EventTargetChainItem::HandleEvent (c:\mozilla\src\dom\events\EventDispatcher.cpp:429)
#04: mozilla::EventTargetChainItem::HandleEventTargetChain (c:\mozilla\src\dom\events\EventDispatcher.cpp:574)
#05: mozilla::EventDispatcher::Dispatch (c:\mozilla\src\dom\events\EventDispatcher.cpp:1156)
#06: FocusBlurEvent::Run (c:\mozilla\src\dom\base\nsFocusManager.cpp:2077)
#07: nsContentUtils::AddScriptRunner (c:\mozilla\src\dom\base\nsContentUtils.cpp:5682)
#08: nsContentUtils::AddScriptRunner (c:\mozilla\src\dom\base\nsContentUtils.cpp:5689)
#09: nsFocusManager::FireFocusOrBlurEvent (c:\mozilla\src\dom\base\nsFocusManager.cpp:2261)
#10: nsFocusManager::SendFocusOrBlurEvent (c:\mozilla\src\dom\base\nsFocusManager.cpp:2215)
#11: nsFocusManager::Blur (c:\mozilla\src\dom\base\nsFocusManager.cpp:1772)
#12: nsFocusManager::SetFocusInner (c:\mozilla\src\dom\base\nsFocusManager.cpp:1392)
#13: nsFocusManager::MoveFocus (c:\mozilla\src\dom\base\nsFocusManager.cpp:587)
#14: nsSelectMoveScrollCommand::DoCommand (c:\mozilla\src\dom\base\nsGlobalWindowCommands.cpp:329)
#15: nsControllerCommandTable::DoCommand (c:\mozilla\src\dom\commandhandler\nsControllerCommandTable.cpp:150)
#16: nsBaseCommandController::DoCommand (c:\mozilla\src\dom\commandhandler\nsBaseCommandController.cpp:134)
#17: nsXBLPrototypeHandler::DispatchXBLCommand (c:\mozilla\src\dom\xbl\nsXBLPrototypeHandler.cpp:584)
#18: nsXBLWindowKeyHandler::WalkHandlersAndExecute (c:\mozilla\src\dom\xbl\nsXBLWindowKeyHandler.cpp:622)
#19: nsXBLWindowKeyHandler::WalkHandlersInternal (c:\mozilla\src\dom\xbl\nsXBLWindowKeyHandler.cpp:478)
#20: nsXBLWindowKeyHandler::WalkHandlers (c:\mozilla\src\dom\xbl\nsXBLWindowKeyHandler.cpp:165)
#21: nsXBLWindowKeyHandler::HandleEvent (c:\mozilla\src\dom\xbl\nsXBLWindowKeyHandler.cpp:359)
#22: mozilla::EventListenerManager::HandleEventSubType (c:\mozilla\src\dom\events\EventListenerManager.cpp:1107)
#23: mozilla::EventListenerManager::HandleEventInternal (c:\mozilla\src\dom\events\EventListenerManager.cpp:1311)
#24: mozilla::EventTargetChainItem::HandleEvent (c:\mozilla\src\dom\events\EventDispatcher.cpp:429)
#25: mozilla::EventTargetChainItem::HandleEventTargetChain (c:\mozilla\src\dom\events\EventDispatcher.cpp:679)
#26: mozilla::EventTargetChainItem::HandleEventTargetChain (c:\mozilla\src\dom\events\EventDispatcher.cpp:725)
#27: mozilla::EventDispatcher::Dispatch (c:\mozilla\src\dom\events\EventDispatcher.cpp:1156)
#28: mozilla::PresShell::DispatchEventToDOM (c:\mozilla\src\layout\base\PresShell.cpp:8023)
#29: mozilla::PresShell::HandleEventInternal (c:\mozilla\src\layout\base\PresShell.cpp:7698)
#30: mozilla::PresShell::HandleEvent (c:\mozilla\src\layout\base\PresShell.cpp:7389)
#31: nsViewManager::DispatchEvent (c:\mozilla\src\view\nsViewManager.cpp:815)

nsSelectMoveScrollCommand causes moving focus, but I'm still not sure what's going on...

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Ah, perpahs, I got it.

We look for a scrollable frame from current selection. The scrollable frame could be outside of selection ancestor limit like bug 1482425. However, nsFrameSelection::CommonPageMove() emulates click outside the selection ancestor limit which is editing host of the editor in this case.
https://searchfox.org/mozilla-central/rev/e0c879c86b95bdc752b1dbff6088169735674e4a/layout/generic/nsFrameSelection.cpp#1841-1842
Then, AdjustFocusAfterCaretMove() move focus from the editor.
https://searchfox.org/mozilla-central/rev/e0c879c86b95bdc752b1dbff6088169735674e4a/dom/base/nsGlobalWindowCommands.cpp#239,245-246

So, perhaps, nsFrameSelection::HandleClick() shouldn't emulate click outside of ancestor limit at least in this case.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ccc743cda128ff226ce0f25e301eaca48590dda9

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5f17fa8e11ce84561e87a03e67bfb044fcaced22

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=12da2e75e4e87488537a6ddd0a76be51a9d32731

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=399aa0f3c466ed8f3fe208df3f1058c201a0fbe2

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1498816 - part 0: Add automated tests of PageDown/PageUp key handling in an editing host

Even if there is no scrollable element is in focused editing host, its parent
scrollable element should be scrolled.  However, focus shouldn't be moved and
selection ranges should be kept in the editing host.

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1498816 - part 1: Make nsFrameSelection::CommonPageMove() emulate click in current selection limiter

nsFrameSelection::CommonPageMove() is called only by
nsTextInputSelectionImpl::PageMove() and PresShell::PageMove().  So, this is
the only implementation of (Shift+) PageDown and (Shift+) PageUp.

This scrolls down/up the specific frame.  However, this allows to scroll
outside of selection limiter, for example, even when an editing host is
focused, its parent scrollable element may be scrolled.  This is same behavior
as Blink so that we should keep this behavior.

However, it also emulates to click same position after scroll and this behavior
is different from Blink.  At this time, it does not check selection limiter and
then, nsFrameSelection::HandleClick() may reset selection limiter the scrolled
frame is a parent frame of the limiter.

Therefore, this patch makes it check if the scrolled frame is a parent of the
limiter, and if so, use result of GetFrameToPageSelect() to emulate a click
instead.  The result won't be a parent of the limiter because it is used when
handling Shift + PageDown and Shift + PageUp which are always handled in the
limiter.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3ea2a2c78bf4
part 0: Add automated tests of PageDown/PageUp key handling in an editing host r=smaug
https://hg.mozilla.org/integration/autoland/rev/0a0c7f90e571
part 1: Make nsFrameSelection::CommonPageMove() emulate click in current selection limiter r=smaug

Comment 14

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/3ea2a2c78bf4
https://hg.mozilla.org/mozilla-central/rev/0a0c7f90e571