Closed Bug 1455496 Opened 6 years ago Closed 6 years ago

Hide menu bar of scratchpad on toolbox.

Categories

(DevTools Graveyard :: Scratchpad, defect)

defect
Not set
normal

Tracking

(firefox-esr52 unaffected, firefox59 unaffected, firefox60 unaffected, firefox61 fixed)

RESOLVED FIXED
Firefox 61
Tracking Status
firefox-esr52 --- unaffected
firefox59 --- unaffected
firefox60 --- unaffected
firefox61 --- fixed

People

(Reporter: mantaroh, Assigned: mantaroh)

References

Details

(Keywords: regression)

Attachments

(3 files, 2 obsolete files)

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 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.
Attachment #8969592 - Flags: review?(dao+bmo) → review+
Assignee: nobody → mantaroh
Blocks: 1453281
Keywords: regression
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)
Attachment #8969593 - Flags: review?(jdescottes) → review+
Attachment #8969592 - Attachment is obsolete: true
Attachment #8969593 - Attachment is obsolete: true
Attachment #8969876 - Flags: review?(dao+bmo)
Attachment #8969877 - Flags: review?(jdescottes)
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
Pushed by mantaroh@gmail.com:
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
https://hg.mozilla.org/mozilla-central/rev/dd464b3666b1
https://hg.mozilla.org/mozilla-central/rev/997646442b81
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
QA Whiteboard: [good first verify]
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
QA Whiteboard: [good first verify] → [good first verify] [bugday-20180516]
Product: Firefox → DevTools
Product: DevTools → DevTools Graveyard
You need to log in before you can comment on or make changes to this bug.