1392873 - all-tabs-menu doesn't work when Inspector is in separate window

Status: VERIFIED FIXED

(DevTools :: Inspector, defect)

Description

[atlanto]

STR:
1. Open Inspector
2. Show it in separate window
3. Click all-tabs-menu(▼)

AR:
Nothing happened.

ER:
All tabs menu(Rules/Computed/Animations/Fonts) opened.

Comment 1

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=ebf65a1af072e0c4d258257477812dc66c8b635d&tochange=0fc0b762b40a72ffdb519167dd5b249a3ea629b2

Regressed by: 0fc0b762b40a	Ricky Chien — Bug 1344155 - Remove toolbox in netmonitor r=Honza


An error is shown in Browser Console when click the dropdown marker of inspector window:

TypeError: popupset is null[Learn More]  menu.js:72:7


:rickychien,
Your patch seems to cause the regression. Could you look this?

Comment 2

[Ricky Chien [:rickychien] (inactive)]

Attachment: Bug 1392873 - Fix missing all-tabs-menu when detaching devtools to a separate window

Review commit: https://reviewboard.mozilla.org/r/171520/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/171520/

Comment 3

[Ricky Chien [:rickychien] (inactive)]

The root cause is because that the popup's document could be changed when detaching devtools panel to a separate window. I'm going to introduce menuDocument props to address this issue.

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8900126 [details]
Bug 1392873 - Fix missing all-tabs-menu when detaching devtools to a separate window

https://reviewboard.mozilla.org/r/171520/#review177418

Thanks for the patch Ricky!

Honza

::: devtools/client/shared/components/tabs/tabbar.js:201
(Diff revision 1)
>      // https://bugzilla.mozilla.org/show_bug.cgi?id=1274551
>      let rect = target.getBoundingClientRect();
>      let screenX = target.ownerDocument.defaultView.mozInnerScreenX;
>      let screenY = target.ownerDocument.defaultView.mozInnerScreenY;
>      menu.popup(rect.left + screenX, rect.bottom + screenY,
> -      { doc: window.parent.document });
> +      { doc: this.props.menuDocument });

What about having fallback to `window.parent.document` in case the `menuDocument` isn't set?

Honza

Comment 5

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8900126 [details]
Bug 1392873 - Fix missing all-tabs-menu when detaching devtools to a separate window

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171520/diff/1-2/

Comment 6

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8900126 [details]
Bug 1392873 - Fix missing all-tabs-menu when detaching devtools to a separate window

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171520/diff/2-3/

Comment 7

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8900126 [details]
Bug 1392873 - Fix missing all-tabs-menu when detaching devtools to a separate window

https://reviewboard.mozilla.org/r/171520/#review177892

Looks good, thanks!

Honza

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5ec1350d5cc3
Fix missing all-tabs-menu when detaching devtools to a separate window r=Honza

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5ec1350d5cc3

Comment 10

[Sayed [:LiquidO2]]

I can reproduce this issue in Nightly following the str from comment 0 with 57.0a1 (2017-08-22) (64-bit) in 64-bit Linux.

I can verify that this bug is fixed in latest Nightly 57.0a1

Build ID 	20170826100418
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0

Comment 11

[Syed Nayeem Roman]

I have successfully reproduced the bug in firefox Nightly 57.0a1 (2017-08-22) (64-bit) with windows 10 (64 bit) 

Verified as fixed with latest nightly 57.0a1 (2017-08-26) (64-bit)


Build ID :	20170826100418
User Agent : 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0

[bugday-20170823]

Comment 12

[Sayed [:LiquidO2]]

This bug is verified in both linux(comment 10) and windows(comment 11). So, I am marking this bug as verified fixed.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this when you get a chance.

Comment 14

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8900126 [details]
Bug 1392873 - Fix missing all-tabs-menu when detaching devtools to a separate window

Approval Request Comment
[Feature/Bug causing the regression]: regression from bug 1344155
[User impact if declined]: usability impact
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: minor
[Why is the change risky/not risky?]: only happen when detaching Devtools panel.
[String changes made/needed]: none

Comment 15

[Gerry Chang [:gchang]]

Comment on attachment 8900126 [details]
Bug 1392873 - Fix missing all-tabs-menu when detaching devtools to a separate window

Fix a regression and was verified. Beta56+.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ec2c40638986