[e10s] Switching, opening, and closing tabs doesn't work while a tab is loading

VERIFIED FIXED in Firefox 39

Status

()

defect
VERIFIED FIXED
5 years ago
3 years ago

People

(Reporter: ttaubert, Assigned: Felipe)

Tracking

(Blocks 2 bugs, {dev-doc-complete})

Trunk
Firefox 39
x86
All
Points:
5
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify +

Firefox Tracking Flags

(e10sm5+, firefox39 verified)

Details

(URL)

Attachments

(3 attachments)

If you open a URL that takes a while to load (e.g. some youtube one for me) then while the spinner is active pressing Cmd+T/W doesn't do anything. You can click a tab to switch to it but the browser pane won't actually switch.

It seems like we don't process events or messages at all?
When switching tabs the content pane becomes white even.
Sounds like a dupe of bug 1067523, although that one got resolved because the site changed.
Blocks: e10s-perf
No longer blocks: e10s
tracking-e10s: --- → ?
Assignee: nobody → wmccloskey
We route keystrokes through the content process before chrome gets a chance to use them. If the content process is busy, then we won't respond to keyboard shortcuts. I think it probably makes sense to avoid sending shortcuts like Ctrl-T/W to content since we probably don't want a web page to be able to intercept those anyway.
That sounds like a solution to bug 1052569, and also an opening of the bug 380637 can of worms (it will break some web apps in some cases).

Comment 5

5 years ago
"You can click a tab to switch to it but the browser pane won't actually switch."
dom.ipc.processCount works well.

Repeating part of comment I just added in bug 1101138. Some keys work, at least Ctrl+Tab and Alt+f.

My hunch [2nd time referring to this] for single processCount fixing bug 666365 may help reduce delay; "the browser pane won't actually switch."

Updated

5 years ago
OS: Mac OS X → All
Flags: firefox-backlog+
Felipe, would you mind taking this over? It's very closely related to bug 862519.
Flags: needinfo?(felipc)
Sure
Assignee: wmccloskey → felipc
Flags: needinfo?(felipc)
In bug 862519 we added e10s support to commands, by allowing the key events to first be dispatched to content, wait for a response, and then process them in the parent with the correct preventedDefault flag.

This adds support to add an attribute (reserved="true") to commands, which will skip that support, making them non overridable by web content. This is helpful in that, when the content is busy, we don't want to wait to process these commands.

The list of reserved commands will be kept short, and as much as possible I think we should mirror Chrome's list of reserved keys (comment 7 has a link to that list) in order to avoid web compat issues.
Attachment #8573306 - Flags: review?(bugs)
As described in comment 4, this should fix bug 1052569 and open the non-overridable commands can of worms.. This reserves:

New Tab
Close Tab
New Window
Close Window
Next Tab
Prev Tab
Quit
New Private Browsing

Which is a conservative list, and a subset of what Chrome reserves, so hopefully web compat issues would be minimal with these. Note however that their Private Browsing shortcut is different than ours, so that is the one more at risk.

This also makes it easy for a simple add-on to reserve more keys like Ctrl+F, which a lot of people want but it's probably harder to argue for it as a default.
Attachment #8573310 - Flags: review?(gavin.sharp)
Hi Felipe, can you provide a point value.
Status: NEW → ASSIGNED
Iteration: --- → 39.1 - 9 Mar
Flags: qe-verify?
Flags: needinfo?(felipc)
Comment on attachment 8573310 [details] [diff] [review]
Reserve main browser commands

Seems worth trying. Philipp might have feedback on the list of shortcuts to block.
Attachment #8573310 - Flags: ui-review?(philipp)
Attachment #8573310 - Flags: review?(gavin.sharp)
Attachment #8573310 - Flags: review+
Comment on attachment 8573306 [details] [diff] [review]
Allow support for reserved commands

This makes HasHandlerForEvent to behave rather oddly.
It doesn't check anymore whether there are handlers for the event, but whether
there are non-reserved handlers for the event, whatever that means.

So, change the name of HasHandlerForEvent at least
And I think "reserved" doesn't quite describe what is happening here, since it is
more like "dont-let-content-process-to-override-the-behavior-but-stuff-in-chrome-can-still-do-that". Could we call the attribute chromeOnlyHandling="true" ?

If we want to make chrome to always execute these handlers, shouldn't we prevent
content to even get the relevant key events. So I'd expect mNoCrossProcessBoundaryForwarding to be set somewhere.
Attachment #8573306 - Flags: review?(bugs) → review-
Comment on attachment 8573310 [details] [diff] [review]
Reserve main browser commands

If that's a subset of what Chrome reserves, it should be fine. I'm slightly concerned about new private window since the P key might be used for print related stuff. But we can include it for now and then remove it later if lots of apps break.

Out of interest: does IE also block certain key commands?
Attachment #8573310 - Flags: ui-review?(philipp) → ui-review+
Ok, this is a much cleaner version of the patch. I was abusing the aExecute param to mean two things at the same time, but now instead I just added a new outParam to HasHandlerForEvent to optionally query if the handler is reserved. With this I think the HasHandlerForEvent name can remain unchanged.

I also set the mNoCrossProcessBoundaryForwarding as requested to make it so that content doesn't even receive it. I had left it out deliberately ("ah, might as well let the page at least receive the event"), but yeah that would be strange behavior.

I just wanted to push back on the name of the attribute. I believe "reserved" in this context makes sense, since it's declared with the XUL command handlers, it's expected that chrome can still do whatever it wants with the event, it's just reserved from being exposed to content. (And it looks a bit cleaner than "reservedForChrome" in the file). But if you still think it's not a good name I can change it when landing.
Attachment #8574785 - Flags: review?(bugs)
Points: --- → 5
Flags: qe-verify?
Flags: qe-verify+
Flags: needinfo?(felipc)
Iteration: 39.1 - 9 Mar → 39.2 - 23 Mar
Attachment #8574785 - Flags: review?(bugs) → review+
This added a new attribute for XUL <command> elements, reserved="true", which makes the key event for that command to not be forwarded to content, making it impossible for content to see or preventDefault those commands. We're currently using it for Ctrl+T, +W, +N, etc.  (Full list at comment 11)

This is an e10s-only feature.
Keywords: dev-doc-needed
https://hg.mozilla.org/mozilla-central/rev/7c05621a38a2
https://hg.mozilla.org/mozilla-central/rev/a5c70a2fab90
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
I was able to reproduce this issue on Firefox 36.0a1 (2014-11-18) using Ubuntu 12.04 64bit and Windows 7 64bit.

Verified fixed on Firefox 39.0a1 (2015-03-16) using Ubuntu 12.04 64 bit, Windows 7 64bit and Mac OS X 10.9.5.
Status: RESOLVED → VERIFIED
I've added a note to Site Compat for Firefox 39:
https://developer.mozilla.org/en-US/Firefox/Releases/39/Site_Compatibility#Reserved_browser_operations

And a page on the new XUL attribute:
https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/Attribute/reserved

Please let me know if this is correct and covers things.
Flags: needinfo?(felipc)
Looks good!
Flags: needinfo?(felipc)
Blocks: 1052569
Blocks: 1203059
No longer depends on: 1203059

Updated

3 years ago
Depends on: 1297342

Updated

3 years ago
Depends on: 1301476

Comment 23

3 years ago
I'm on the Google Chrome team and we are now thinking that sites should be allowed to preventDefault on keyboard shortcuts while in fullscreen mode (though we haven't implemented yet). Edge already does this. Wondering if Firefox team would be willing to change this behaviour, and if the reason for changing to a non-interceptable model was a policy decision or technical limitation (as suggested by this bug).

Am I right in understanding that the key combinations on the reserved list (which aren't delivered to the site) are the only ones that can be used while the site is loading? (So there is an architectural limitation to allowing sites to capture the events, not just a policy decision.)
You need to log in before you can comment on or make changes to this bug.