1877019 - Shift key affects window.open for 5 seconds.

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[knoa]

(It depends on the bug 1873330 - Open a new window if shift key is held on window.open call)

Background:
I have my custom shortcut key [G] for quick googling, which googles the keyword(s) with the selected text on the web page on a new tab by window.open.
For more convenience, I use [Shift + G] to add/edit the selected text with a window.prompt input.

Problem:
On recent Firefoxes, maybe from Firefox 123, [Shift + G] opens a whole new window, not a tab.
It seems to refer to the shift key to open a new window instead of the new tab.
Although I do press the shift key down to fire the [Shift + G] shortcut key, the shift key is just the command to launch the window.prompt, not for alternating the window.open functionality. Actual window.open fires way after the shift key itself.
But if you wait 5 seconds after the prompt is shown, the shift key won't affect the window.open.

Proposal:
Check the shift key status at the very moment before window.open.
Or can I change the "5 seconds" threshold on about:config?

How to reproduce 1:

1. Run the Test 1 below on the console.
   2-1. Press [Shift + G] to launch the window.prompt.
   2-2. Type some keywords within 5 seconds, and press [Enter].
   2-3. Firefox opens a new window.
2. Back to the original tab.
   4-1. Press [Shift + G] to launch the window.prompt.
   4-2. Type some keywords, wait more than 5 seconds, and press [Enter].
   4-3. Firefox opens a new tab.

/* Test 1: an in-real-life example /
/ You can launch it with [Shift + G]. /
(function(){
document.addEventListener('keydown', e => {
console.log(e.key, e.shiftKey);
if(e.key === 'G'/ e.key has capital letters with shiftKey */){
const selection = document.getSelection(), keyword = selection.toString();
const startTime = Date.now();
const q = window.prompt('Google:', keyword + ' ');
if(q === '' || q === null) return;
window.open('https://www.google.com/search?q=' + encodeURIComponent(q));
const endTime = Date.now();
console.log(endTime - startTime, 'ms');
}
});
})();

How to reproduce 2:

1. Run the Test 2 below on the console.
2. Press [Shift + X] to launch the window.open commands.
   3-1. Firefox immediately opens a new window(0).
   3-2. Firefox opens a new window(1000) after 1000ms.
   3-3. Firefox opens a new tab(6000) after 6000ms.

/* Test 2: bare minimum /
/ It opens 3 windows/tabs, 0ms, 1000ms, 6000ms later on each. The former two would open as new windows, whereas the 6000ms opens as a new tab. /
(function(){
document.addEventListener('keydown', e => {
console.log(e.key, e.shiftKey);
if(e.key === 'X'/ e.key has capital letters with shiftKey */){
window.open('https://www.google.com/search?q=' + encodeURIComponent(0));
setTimeout(() => window.open('https://www.google.com/search?q=' + encodeURIComponent(1000)), 1000)
setTimeout(() => window.open('https://www.google.com/search?q=' + encodeURIComponent(6000)), 6000)
}
});
})();

Comment 1

[knoa]

(I should've used Markdown... sorry for the inconvenience)

Comment 2

[knoa]

Even if it messes up, I repost it with Markdown.

(It depends on the bug 1873330 - Open a new window if shift key is held on window.open call)

Background:

I have my custom shortcut key [G] for quick googling, which googles the keyword(s) with the selected text on the web page on a new tab by window.open.
For more convenience, I use [Shift + G] to add/edit the selected text with a window.prompt input.

Problem:

On recent Firefoxes, maybe from Firefox 123, [Shift + G] opens a whole new window, not a tab.
It seems to refer to the shift key to open a new window instead of the new tab.
Although I do press the shift key down to fire the [Shift + G] shortcut key, the shift key is just the command to launch the window.prompt, not for alternating the window.open functionality. Actual window.open fires way after the shift key itself.
But if you wait 5 seconds after the prompt is shown, the shift key won't affect the window.open.

Proposal:

Check the shift key status at the very moment before window.open.
Or can I change the "5 seconds" threshold on about:config?

How to reproduce 1:

1. Run the Test 1 below on the console.

• 2-1. Press [Shift + G] to launch the window.prompt.
• 2-2. Type some keywords within 5 seconds, and press [Enter].
• 2-3. Firefox opens a new window.

3. Back to the original tab.

• 4-1. Press [Shift + G] to launch the window.prompt.
• 4-2. Type some keywords, wait more than 5 seconds, and press [Enter].
• 4-3. Firefox opens a new tab.

/* Test 1: an in-real-life example */
/* You can launch it with [Shift + G]. */
(function(){
  document.addEventListener('keydown', e => {
    console.log(e.key, e.shiftKey);
    if(e.key === 'G'/* e.key has capital letters with shiftKey */){
      const selection = document.getSelection(), keyword = selection.toString();
      const startTime = Date.now();
      const q = window.prompt('Google:', keyword + ' ');
      if(q === '' || q === null) return;
      window.open('https://www.google.com/search?q=' + encodeURIComponent(q));
      const endTime = Date.now();
      console.log(endTime - startTime, 'ms');
    }
  });
})();

How to reproduce 2:

1. Run the Test 2 below on the console.
2. Press [Shift + X] to launch the window.open commands.

• 3-1. Firefox immediately opens a new window(0).
• 3-2. Firefox opens a new window(4000) after 4000ms.
• 3-3. Firefox opens a new tab(6000) after 6000ms.

/* Test 2: bare minimum */
/* It opens 3 windows/tabs, 0ms, 4000ms, 6000ms later on each. The former two would open as new windows, whereas the 6000ms opens as a new tab. */
(function(){
  document.addEventListener('keydown', e => {
    console.log(e.key, e.shiftKey);
    if(e.key === 'X'/* e.key has capital letters with shiftKey */){
      window.open('https://www.google.com/search?q=' + encodeURIComponent(0));
      setTimeout(() => window.open('https://www.google.com/search?q=' + encodeURIComponent(4000)), 4000)
      setTimeout(() => window.open('https://www.google.com/search?q=' + encodeURIComponent(6000)), 6000)
    }
  });
})();

Comment 3

[Tooru Fujisawa [:arai]]

Thank you for reporting.

Just to make sure, how is the keyboard shortcut implemented in actual case?
I suppose evaluating the code in console is only for the demonstrating the issue, and it's different than the actual case's scenario. is it correct?

Is it done with WebExtensions, or bookmarklet, or something else?
I wonder how/why window.open after 6000ms (which is, after user activation expired) is allowed in your case.

Comment 4

[knoa]

Thank you for reviewing the post.

My actual use case is by Shortkeys addon.
https://addons.mozilla.org/ja/firefox/addon/shortkeys/

I assign [G] for this: (it works well without any issue)

// window.open().location.assign() is for dealing with Wayback Machine which overwrites the open method with Wombat.js.
(function(){
  const selection = document.getSelection(), keyword = selection.toString();
  const q = (keyword !== '') ? keyword : window.prompt('Google:', '');
  if(q === null) return;
  if(q !== ''){
    if(q.match(/^https?:\/\//)) window.open().location.assign(encodeURI(q));
    else window.open().location.assign('https://www.google.co.jp/search?q=' + encodeURIComponent(q));
  }
})();

I also assign [shift+G] for this: (it opens a new window if it takes less than 5 seconds)

(function(){
  const selection = document.getSelection(), keyword = selection.toString();
  const q = window.prompt('Google:', keyword + ' ');
  if(q === null) return;
  if(q !== ''){
    if(q.match(/^https?:\/\//)) window.open().location.assign(encodeURI(q));
    else window.open().location.assign('https://www.google.co.jp/search?q=' + encodeURIComponent(q));
  }
})();

By the way, doesn't window.open after 6000ms work with your environment?
I tested the setTimeout code on my Firefox Nightly 124.0a1, which is almost in vanilla condition.

Comment 5

[Tooru Fujisawa [:arai]]

Thank you for the information.

Yeah, it opens window after 6000ms if I evaluate the code in console, but that shouldn't be the case if it's evaluated with web content's privilege, given it will be blocked due to lack of user activation.

And yes, WebExtensions' context can be different than web content's case, I'll look into how the window.open and its privilege is handled in that context.
maybe we can make the behavior applied only to web content, or maybe to specific event details that acts as "open this link", possibly excluding regular keyboard events except for Enter etc.

Comment 6

[knoa]

Thank you.
I assume it can be fixed by some contextual privilege modification, as you mentioned.
But from my naive perspective, it seems to be the simpler way: Check the shift key status at the very moment before window.open.
Maybe not...?

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1873330

Comment 8

[Bob Hood [:bhood]]

:arai, could you please apply Priority/Severity ratings to this report when you get a chance?

Comment 9

[Tooru Fujisawa [:arai]]

Attachment: Bug 1877019 - Reflect user activation modifiers for keyboard event only if it likely activates a link or a link-like button. r?smaug!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d9bb3d2e64d0
Reflect user activation modifiers for keyboard event only if it likely activates a link or a link-like button. r=smaug

Comment 11

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/d9bb3d2e64d0

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:arai, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval. Also, don't forget to request an uplift for the patches in the regression caused by this fix.
• If no, please set status-firefox123 to wontfix.

For more information, please visit BugBot documentation.

Comment 13

[Tooru Fujisawa [:arai]]

Attachment: Bug 1877019 - Reflect user activation modifiers for keyboard event only if it likely activates a link or a link-like button. r?smaug!

Original Revision: https://phabricator.services.mozilla.com/D200202

Comment 14

[Tooru Fujisawa [:arai]]

Attachment: Bug 1877019 - Reflect user activation modifiers for keyboard event only if it likely activates a link or a link-like button. r?smaug!

Original Revision: https://phabricator.services.mozilla.com/D200202

Comment 15

[Phabricator Automation]

Uplift Approval Request

• Explanation of risk level: This disables edge case where new behavior is added by bug 1873330 change
• Needs manual QE test: no
• String changes made/needed: None
• Steps to reproduce for manual QE testing: None
• Is Android affected?: no
• Risk associated with taking this patch: low
• Fix verified in Nightly: yes
• User impact if declined: window.open triggered inside keyboard shortcut gets opened in unexpected place depending on the modifier keys for the shortcut itself
• Code covered by automated testing: yes

Comment 16

[Pascal Chevrel:pascalc]

Tooru, it seems that this patch does not apply to the beta branch.

Comment 17

[Tooru Fujisawa [:arai]]

looks like there's change from bug 1780071 in between them.
I'll address it shortly

Comment 18

[Pascal Chevrel:pascalc]

Comment on attachment 9378241 [details]
Bug 1877019 - Reflect user activation modifiers for keyboard event only if it likely activates a link or a link-like button. r?smaug!

Tooru, this is a P2/S4 and we are shipping our last betas, it seems like unnecessary risk to uplift at this point in the cycle, let's have it ship with 124. Thanks.