1455496 - Hide menu bar of scratchpad on toolbox.

Status: RESOLVED FIXED

(DevTools Graveyard :: Scratchpad, defect)

Description

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: Move the window using by menu bar of scratchpad.

Current scratchpad of toolbox display the menu bar.
Especially, we can move the window on GTK environment with this toolbar.

STR:
 1. Open devtool
 2. Add scratchpad panel from the settings menu.(i.e. "settings" in the meatball)
 3. Open scratchpad panel.

AR: scratchpad has a menu bar.

ER: menu bar will not be displayed on a toolbox.

Comment 1

[Mantaroh Yoshinaga[:mantaroh]]

Scratchpad panel should be disabled toolbar instead of the menubar. [1][2]

[1] https://searchfox.org/mozilla-central/rev/11a2ae294f50049e12515b5821f5a396d951aacb/devtools/client/scratchpad/scratchpad.js#340
[2] https://searchfox.org/mozilla-central/rev/11a2ae294f50049e12515b5821f5a396d951aacb/devtools/client/scratchpad/scratchpad.xul#144

Comment 2

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: Bug 1455496 - Part 1. Hide toolbar of scratchpad panel.

The scratchpad panel(in the toolbox) should not display menu bar.
This patch will hide this menu bar if target is toolbox.

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

Comment 3

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: Bug 1455496 - Part 2. Add test of existence of scratchpad's menu bar.

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

Comment 4

[Dão Gottwald [:dao]]

Comment on attachment 8969592 [details]
Bug 1455496 - Part 1. Hide toolbar of scratchpad panel.

https://reviewboard.mozilla.org/r/238356/#review244144

::: devtools/client/scratchpad/scratchpad.xul:145
(Diff revision 1)
>         modifiers="accel"/>
>  
>  </keyset>
>  
> -<toolbar type="menubar">
> +<toolbar type="menubar" id="sp-menu-toolbar">
>  <menubar id="sp-menubar">

It looks like this id is now unused and can be removed.

Comment 5

[Julian Descottes [:jdescottes]]

Comment on attachment 8969593 [details]
Bug 1455496 - Part 2. Add test of existence of scratchpad's menu bar.

https://reviewboard.mozilla.org/r/238358/#review244146

LGTM, thanks! Would just like to find another name for the test.

::: devtools/client/scratchpad/test/browser.ini:48
(Diff revision 1)
>  [browser_scratchpad_tab_switch.js]
>  [browser_scratchpad_ui.js]
>  [browser_scratchpad_close_toolbox.js]
>  [browser_scratchpad_remember_view_options.js]
>  [browser_scratchpad_disable_view_menu_items.js]
> +[browser_scratchpad_toolbox.js]

This test is really about testing the "display of the menu bar", both in window and panel mode. Could we rename the test to reflect that?

::: devtools/client/scratchpad/test/browser_scratchpad_toolbox.js:20
(Diff revision 1)
> +
> +  info("Open scratchpad.");
> +  let [win] = await openTabAndScratchpad(options);
> +
> +  let menuToolbar = win.document.getElementById("sp-menu-toolbar");
> +  ok(menuToolbar, "The scrachpad should have a menu bar.");

nit: scrachpad -> scratchpad 

(same comment on L35)

Comment 6

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: Bug 1455496 - Part 1. Hide toolbar of scratchpad panel.

The scratchpad panel(in the toolbox) should not display menu bar.
This patch will hide this menu bar if target is toolbox.

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

Comment 7

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: Bug 1455496 - Part 2. Add test of existence of scratchpad's menu bar.

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

Comment 8

[Mantaroh Yoshinaga[:mantaroh]]

Sorry, pushing mq patch looks like to remove the MozReview-Commit-ID. So I removed the r? flag since this patches has granted already. I'll land patch to m-i after checking the try result.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2493e87c35d2fef95729cc3796b5264d3821e5ca

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd464b3666b1
Part 1. Hide toolbar of scratchpad panel. r=dao
https://hg.mozilla.org/integration/mozilla-inbound/rev/997646442b81
Part 2. Add test of existence of scratchpad's menu bar. r=jdescottes

Comment 10

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/dd464b3666b1
https://hg.mozilla.org/mozilla-central/rev/997646442b81

Comment 11

[Md Maruf Rahman]

I have reproduced this bug with Nightly 61.0a1 (2018-04-19)on Windows 10, 64 Bit!

This bug's fix is verified with latest Beta!

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