1519087 - [Ubuntu] - DevTools Settings cannot be cancelled if searchbox is active

Status: VERIFIED FIXED

(DevTools :: General, defect, P3)

Description

[Cristian Fogel [:cfogel]]

Attachment: 2019-01-10 15-02-50.mp4

Affected versions

• 66.0a1 (2019-01-09)

Affected platforms

• Ubuntu 16.04

Steps to reproduce

1. Launch Firefox, open devTools, Storage tab;
2. Click inside the Filter items box;
3. Press on the F1 key;
4. Press on the F1 key again;

Expected result

• the settings tab is no longer displayed, Storage section is once again visible;

Actual result

• the settings tab/view flickers and remains displayed;

Regression range

• bug 1409456 might be at fault, according to mozregression results

Additional notes

• Windows and mac OS don't seem to be affected;
• attached recording with the issue;
• any other tab and search box has the same issue (inspector, network, etc.)

Comment 1

[Cristian Fogel [:cfogel]]

• corrected summary;
• @Julian, mind taking a look at this and confirming if it's the case? Thank you!

Comment 2

[Julian Descottes [:jdescottes]]

Thanks for the investigation Cristi. Looking at bug 1409456, this could be linked to the preventDefault I removed:

--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -818,20 +818,17 @@ Toolbox.prototype = {
       // Flip back to the last used panel if we are already
       // on the options panel.
       if (this.currentToolId === "options" &&
           gDevTools.getToolDefinition(this.lastUsedToolId)) {
         this.selectTool(this.lastUsedToolId);
       } else {
         this.selectTool("options");
       }
-      // Prevent the opening of bookmarks window on toolbox.options.key
-      event.preventDefault();
     };
-    this.shortcuts.on(L10N.getStr("toolbox.options.key"), selectOptions);
     this.shortcuts.on(L10N.getStr("toolbox.help.key"), selectOptions);
   },

The associated comment made me believe it was only useful for the shortcut I was removing, which seemed to have a conflict with the bookmarks window. Maybe as a side effect it was also preventing the bug reported here.

Currently building on Linux to confirm.

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: fix-to-test.patch

My Linux virtual machine is not really working out... Attaching a patch here and will see if anyone on Linux can help test it against the STRs.

Comment 4

[Julian Descottes [:jdescottes]]

:nchevobbe was kind enough to test the patch on Linux and it seems to fix the issue.
However this should be accompanied by a test before landing.

Comment 5

[Tim Spurway [:tspurway]]

:pbro, is this still intended to be uplifted for beta 66?

Comment 6

[Patrick Brosset <:pbro>]

No I don't think we need to worry about uplifting. This is a P3 linux-only bug. I'm updating the tracking flags.

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

Are we still planning on landing this in 67?

Comment 8

[Julian Descottes [:jdescottes]]

No it is a minor bug, updating the flags.

Comment 9

[Neha Kochar [:neha]]

Bulk change for all regression bugs with status-firefox67 as 'fix-optional' to be marked 'affected' for status-firefox68.

Comment 10

[Patrick Brosset <:pbro>]

Updating the flags to signal that we don't need to rush into fixing this in 68.
That said, the patch is ready, and tested on linux, Julian: can we land it?

Comment 11

[Julian Descottes [:jdescottes]]

Attachment: Bug 1519087 - Use preventDefault() on DevTools settings shortcut

Comment 12

[Julian Descottes [:jdescottes]]

I can't find a way to write a test that verifies the fix and I also don't have access to a Linux setup. Let's go for one last try push https://treeherder.mozilla.org/#/jobs?repo=try&revision=c0426c8d42d6e89ac75c6e85032a2f0fe6d8fc14

Otherwise, if I get confirmation the issue is still valid on linux I'll move on to land the fix without a test.

Comment 13

[Julian Descottes [:jdescottes]]

Attachment: Bug 1519087 - Add test for Devtools Settings toggle bug on Linux

Depends on D31358

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fb21a998b327
Use preventDefault() on DevTools settings shortcut r=nchevobbe

Comment 15

[Stefan Hindli [:stefan_hindli]]

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

Comment 16

[Cristian Fogel [:cfogel]]

Fix verified with 68.0a1 (2019-05-19) - Ubuntu 16.04.