Closed Bug 1519087 Opened 10 months ago Closed 6 months ago

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

Categories

(DevTools :: General, defect, P3, minor)

66 Branch
Desktop
Linux
defect

Tracking

(firefox-esr60 unaffected, firefox65 unaffected, firefox66 wontfix, firefox67 wontfix, firefox68 verified)

VERIFIED FIXED
Firefox 68
Tracking Status
firefox-esr60 --- unaffected
firefox65 --- unaffected
firefox66 --- wontfix
firefox67 --- wontfix
firefox68 --- verified

People

(Reporter: cfogel, Assigned: jdescottes)

References

Details

(Keywords: regression)

Attachments

(2 files, 2 obsolete files)

Attached video 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.)
  • corrected summary;
  • @Julian, mind taking a look at this and confirming if it's the case? Thank you!
Flags: needinfo?(jdescottes)
Summary: [Ubuntu] - DevTools Settings cannot be cancelled if → [Ubuntu] - DevTools Settings cannot be cancelled if searchbox is active

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.

Attached patch fix-to-test.patch (obsolete) — Splinter Review

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.

: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.

Flags: needinfo?(jdescottes)
Blocks: 1409456
Has Regression Range: --- → yes
Priority: -- → P3

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

Flags: needinfo?(pbrosset)

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

Flags: needinfo?(pbrosset)

Are we still planning on landing this in 67?

Flags: needinfo?(jdescottes)

No it is a minor bug, updating the flags.

Flags: needinfo?(jdescottes)

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

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?

Flags: needinfo?(jdescottes)
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Flags: needinfo?(jdescottes)
Attachment #9036537 - Attachment is obsolete: true

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.

Depends on D31358

Attachment #9065446 - Attachment is obsolete: true
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fb21a998b327
Use preventDefault() on DevTools settings shortcut r=nchevobbe
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68

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

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.