Create Help menu for Scratchpad

RESOLVED FIXED in Firefox 13

Status

()

Firefox
Developer Tools: Scratchpad
P3
normal
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: rc, Assigned: Johan C)

Tracking

({dev-doc-complete})

Trunk
Firefox 13
dev-doc-complete
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good-first-bugs][fixed-in-fx-team])

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

6 years ago
The Workspace (new feature) should have a Help menu + item which takes a user to a documentation page on MDN, also to be created.

Comment 1

6 years ago
Marking this dev-doc-needed so that we can work with the doc folks to get a doc page created.

The feature page is here:

https://wiki.mozilla.org/DevTools/Features/Workspaces
Keywords: dev-doc-needed
(Reporter)

Updated

6 years ago
Summary: Create Help menu for workspace → Create Help menu for Scratchpad
(Reporter)

Updated

6 years ago
Whiteboard: [good-first-bugs]
Which items should be in Help menu?
(Assignee)

Comment 3

6 years ago
Created attachment 582304 [details] [diff] [review]
patch 0.1

Currently "F1" opens the documentation page on Windows. After a discussion with Rob the equivalent keyboard shortcut on mac (accel + ?) usually only opens the help menu. Any feedback on this?
(Assignee)

Updated

6 years ago
Attachment #582304 - Flags: feedback?(rcampbell)
(Reporter)

Comment 4

6 years ago
Comment on attachment 582304 [details] [diff] [review]
patch 0.1

yeah, I think this looks good.
Attachment #582304 - Flags: feedback?(rcampbell) → feedback+
(Assignee)

Updated

6 years ago
Assignee: nobody → johan.charlez
Triaging. Filter on PEGASUS.

This is a feature that would help newcomers to Scratchpad, especially when we have more features built into it. A quick link to MDN would be nice.
Status: NEW → ASSIGNED
Priority: -- → P3
PEGASUS
Component: Developer Tools → Developer Tools: Scratchpad
QA Contact: developer.tools → developer.tools.scratchpad
(Assignee)

Comment 7

6 years ago
Created attachment 593672 [details] [diff] [review]
patch 0.2

+ focus browser and MDN page
+ test
Attachment #582304 - Attachment is obsolete: true
Attachment #593672 - Flags: feedback?(rcampbell)
(Reporter)

Updated

6 years ago
Assignee: johan.charlez → nobody
Version: unspecified → Trunk
(Reporter)

Comment 8

6 years ago
Comment on attachment 593672 [details] [diff] [review]
patch 0.2

+++ b/browser/devtools/scratchpad/test/Makefile.in
@@ -63,6 +63,7 @@
 		browser_scratchpad_bug690552_display_outputs_errors.js \
 		browser_scratchpad_bug650345_find_ui.js \
 		browser_scratchpad_bug714942_goto_line_ui.js \
+    browser_scratchpad_bug_650760_help_key.js \
 		head.js \

in your makefile, indentation is important. Makefiles like tabs.

in your test, rather than doing the linkClicked check in executeSoon blocks, I would register a listener for TabSelect and do your test in there.

e.g.,

win.gBrowser.tabContainer.addEventListener("TabSelect", tabOpenedTest, false);

Also, I would pull the correct key and accel combos out of the menuitem itself. You can do that via

let keyid = document.getElementById("sp-menu-documentation");
let key = document.getElemementById(keyid);

and do a lookup on the key's attributes to find any additional keyboard modifiers.
(Assignee)

Comment 9

6 years ago
Created attachment 595185 [details] [diff] [review]
patch 0.3

+ reworked my code in 'scratchpad.xul'
+ pull key/keycode & modifiers from menuitem
- don't rely on executeSoon()
Attachment #593672 - Attachment is obsolete: true
Attachment #593672 - Flags: feedback?(rcampbell)
Attachment #595185 - Flags: feedback?(rcampbell)
(Assignee)

Updated

6 years ago
Assignee: nobody → johan.charlez
(Reporter)

Comment 10

6 years ago
Comment on attachment 595185 [details] [diff] [review]
patch 0.3

+    let newTab = this.gBrowser.addTab(this.strings.GetStringFromName("help.openDocumentationPage"));

This line's too long. We try to keep them under 80 characters.

Maybe add a local variable for the uri string.

you have some whitespace on empty lines and in your test code:

e.g.,

+  content.location = "data:text/html,Test keybindings for opening Scratchpad MDN Documentation, bug 650760";
+  gBrowser.selectedBrowser.addEventListener("load", function onTabLoad() {
+    gBrowser.selectedBrowser.removeEventListener("load", onTabLoad, true);
+    /// here
+    ok(window.Scratchpad, "Scratchpad variable exists");
+    /// here
+    openScratchpad(runTest);
+  }, ...

for this section:

+  
+  var aEvent = {
+    shiftKey: false,
+    ctrlKey: false,
+    altKey: false,
+    metaKey: false,
+    accelKey: false
+  }
+  
+  if (modifiers.match("shift"))
+    aEvent.shiftKey = true;
+  

you could do something like:

+  var aEvent = {
+    shiftKey: modifiers.match("shift"),

and do away with all the if checking.

you don't really need the extra isPageOpen function. You could just use:

ok(linkClicked, "MDN page will open);

and then call finishTest();

f+ with those fixes! Lookin' good.
Attachment #595185 - Flags: feedback?(rcampbell) → feedback+
(Assignee)

Comment 11

6 years ago
Created attachment 595758 [details] [diff] [review]
patch 0.4
Attachment #595185 - Attachment is obsolete: true
Attachment #595758 - Flags: review?(rcampbell)
(Reporter)

Comment 12

6 years ago
you are going to love this.

Testing this patch, Cmd-Shift-/ is activating the "Apple" menu. It's probably looking for some non-existent overlay.

Let's get rid of the extra settings and just use the F1 key on all platforms. I will cancel this review request.
(Reporter)

Updated

6 years ago
Attachment #595758 - Flags: review?(rcampbell)
(Assignee)

Comment 13

6 years ago
Created attachment 596314 [details] [diff] [review]
patch 0.5
Attachment #595758 - Attachment is obsolete: true
Attachment #596314 - Flags: review?(rcampbell)
(Reporter)

Comment 14

6 years ago
Comment on attachment 596314 [details] [diff] [review]
patch 0.5

looks good, I think this'll do it.

Thanks Johan!
Attachment #596314 - Flags: review?(rcampbell) → review+
(Reporter)

Updated

6 years ago
Whiteboard: [good-first-bugs] → [good-first-bugs][land-in-fx-team]
(Reporter)

Comment 15

6 years ago
https://hg.mozilla.org/integration/fx-team/rev/3d77924fb970
Whiteboard: [good-first-bugs][land-in-fx-team] → [good-first-bugs][fixed-in-fx-team]
(Reporter)

Comment 16

6 years ago
https://hg.mozilla.org/mozilla-central/rev/3d77924fb970
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 13
Documented:

https://developer.mozilla.org/en/Tools/Scratchpad#Using_Scratchpad
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.