Closed Bug 1388832 Opened 2 years ago Closed 2 years ago

Right-clicking on the Search box within the overflow panel causes 'TypeError: parent.getAttribute is not a function'

Categories

(Firefox :: Toolbars and Customization, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 57
Iteration:
57.3 - Sep 19
Tracking Status
firefox57 --- verified

People

(Reporter: jaws, Assigned: sfoster)

References

(Blocks 1 open bug)

Details

(Whiteboard: [reserve-photon-structure])

Attachments

(1 file)

STR:
Add the Search box to the overflow panel
Right-click just outside of the textbox for the search box when the overflow panel is open

Expected:
See a context menu that allows to "Unpin from Overflow Menu"

Actual:
See a context menu that includes "Pin to Overflow Menu" among other confusing entries

This shows up in the Browser Console:
TypeError: parent.getAttribute is not a function browser.js:5483:11
           onViewToolbarsPopupShowing chrome://browser/content/browser.js:5483:11
           onpopupshowing chrome://browser/content/browser.xul:1:1

The line in question is:
> parent.getAttribute("overflowfortoolbar") || // Needs to work in the overflow list as well.

This is at http://searchfox.org/mozilla-central/rev/0f16d437cce97733c6678d29982a6bcad49f817b/browser/base/content/browser.js#5481
Flags: qe-verify+
Priority: -- → P3
QA Contact: gwimberly
Whiteboard: [photon-structure][triage] → [reserve-photon-structure]
Blocks: 1387512
Priority: P3 → P4
Assignee: nobody → sfoster
Status: NEW → ASSIGNED
Iteration: --- → 57.3 - Sep 19
Priority: P4 → P1
Comment on attachment 8910033 [details]
Bug 1388832 - Fix right-click on searchbar when in overflow menu.

https://reviewboard.mozilla.org/r/181504/#review186818
Attachment #8910033 - Flags: review?(jaws) → review+
Woop. I'm so glad that loop has gone. Thanks Sam!
Pushed by sfoster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6de16b8f63dc
Fix right-click on searchbar when in overflow menu. r=jaws
Backed out for failing browser-chrome's browser/components/customizableui/test/browser_customization_context_menus.js:

https://hg.mozilla.org/integration/autoland/rev/f39c3b2da2d12890a0c1d11b217a90f22fb1d9ce

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=6de16b8f63dcd2667b307ccb9d2c44783fb646f4&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=132267782&repo=autoland

[task 2017-09-20T18:06:26.767Z] 18:06:26     INFO - TEST-START | browser/components/customizableui/test/browser_customization_context_menus.js
[task 2017-09-20T18:06:26.851Z] 18:06:26     INFO - TEST-INFO | started process screentopng
[task 2017-09-20T18:06:28.678Z] 18:06:28     INFO - TEST-INFO | screentopng: exit 0
[task 2017-09-20T18:06:28.680Z] 18:06:28     INFO - Buffered messages logged at 18:06:26
[task 2017-09-20T18:06:28.682Z] 18:06:28     INFO - Entering test bound home_button_context
[task 2017-09-20T18:06:28.684Z] 18:06:28     INFO - TEST-PASS | browser/components/customizableui/test/browser_customization_context_menus.js | menuitem should match .customize-context-moveToPanel selector - 
[task 2017-09-20T18:06:28.690Z] 18:06:28     INFO - Buffered messages finished
[task 2017-09-20T18:06:28.693Z] 18:06:28     INFO - TEST-UNEXPECTED-FAIL | browser/components/customizableui/test/browser_customization_context_menus.js | disabled state for .customize-context-moveToPanel - Got true, expected false
[task 2017-09-20T18:06:28.695Z] 18:06:28     INFO - Stack trace:
[task 2017-09-20T18:06:28.700Z] 18:06:28     INFO - chrome://mochikit/content/browser-test.js:test_is:1011
[task 2017-09-20T18:06:28.704Z] 18:06:28     INFO - chrome://mochitests/content/browser/browser/components/customizableui/test/head.js:checkContextMenu:505
[task 2017-09-20T18:06:28.706Z] 18:06:28     INFO - chrome://mochitests/content/browser/browser/components/customizableui/test/browser_customization_context_menus.js:home_button_context:36
Flags: needinfo?(sfoster)
Turns out in onViewToolbarsPopupShowing, we are looking for an element whose *parent* is one of .customization-target, [overflowfortoolbar], toolbarpaletteitem, toolbar. That implies something like elem.closest(".customization-target > *, [overflowfortoolbar] > *, toolbarpaletteitem > *, toolbar > *") which tips back the balance in favor of the original while loop. 

New patch pushed, and try runs triggered: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6ea642858bce03466def9b3ba1ba24248fe98b18
Flags: needinfo?(sfoster)
Comment on attachment 8910033 [details]
Bug 1388832 - Fix right-click on searchbar when in overflow menu.

https://reviewboard.mozilla.org/r/181504/#review187264

::: browser/base/content/browser.js:5433
(Diff revision 2)
> +      if (parent.nodeType !== Node.ELEMENT_NODE) {
> +        break;
> +      }
>        if ((parent.classList && parent.classList.contains("customization-target")) ||

Please combine this with the if-conditional below it.

> if (parent.nodeType != Node.ELEMENT_NODE ||
>     (parent.classList && parent.classList.contains("customization-target")) ||
>     ...
Comment on attachment 8910033 [details]
Bug 1388832 - Fix right-click on searchbar when in overflow menu.

Clearing r+ as original patch was backed out
Attachment #8910033 - Flags: review+
Comment on attachment 8910033 [details]
Bug 1388832 - Fix right-click on searchbar when in overflow menu.

Just moved the parent.nodeType !== Node.ELEMENT_NODE check inside the if() that followed it.
Attachment #8910033 - Flags: review?(jaws)
Comment on attachment 8910033 [details]
Bug 1388832 - Fix right-click on searchbar when in overflow menu.

https://reviewboard.mozilla.org/r/181504/#review187288
Attachment #8910033 - Flags: review?(jaws) → review+
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/72881ce767fc
Fix right-click on searchbar when in overflow menu. r=jaws
https://hg.mozilla.org/mozilla-central/rev/72881ce767fc
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
I have reproduced the issue mentioned in comment 0, using an affected Firefox 57.0a1 build (BuildId:20170809100326).

I have verified that the issue is not reproducible using Firefox 57.0b6 (Build Id:20171005195903) on Windows 10 64bit, macOS 10.11.6 and Ubuntu 16.04 64bit.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.