Open Bug 713192 Opened 12 years ago Updated 2 years ago

test_contextmenu.html: fix some nits I noticed while working on bug 712871

Categories

(Firefox :: General, defect)

defect

Tracking

()

Firefox 12

People

(Reporter: sgautherie, Unassigned)

References

()

Details

Attachments

(1 file, 1 obsolete file)

      No description provided.
Comment on attachment 584025 [details] [diff] [review]
(Av1-FF) Improve code
[Checked in: See comment 7]

>diff --git a/browser/base/content/test/test_contextmenu.html b/browser/base/content/test/test_contextmenu.html

>-        checkContextMenu(["context-copyemail", true].concat(inspectItems));
>+        checkContextMenu(["context-copyemail", true
>+                         ].concat(inspectItems));

>-                               "context-viewframeinfo",     true], null].concat(inspectItems));
>+                               "context-viewframeinfo",     true], null
>+                         ].concat(inspectItems));

>-                               "context-viewframeinfo",     true], null].concat(inspectItems));
>+                               "context-viewframeinfo",     true], null
>+                         ].concat(inspectItems));

>-                               "spell-add-dictionaries",       true], null,
>+                               "spell-add-dictionaries",       true], null
>                          ].concat(inspectItems));

Please don't make these changes.
Attachment #584025 - Flags: review?(gavin.sharp) → review+
Attached patch (Av1a-FF) Improve code (obsolete) — Splinter Review
Av1-FF, with comment 2 suggestion(s),
and more tweaks.


(In reply to Gavin Sharp (use gavin@gavinsharp.com for email) from comment #2)

> >-        checkContextMenu(["context-copyemail", true].concat(inspectItems));
> >+        checkContextMenu(["context-copyemail", true
> >+                         ].concat(inspectItems));

These 3: I was just sync'ing with the other 16.
But as you prefer.

> >-                               "spell-add-dictionaries",       true], null,
> >+                               "spell-add-dictionaries",       true], null
> >                          ].concat(inspectItems));
> 
> Please don't make these changes.

This one (= two, now): I think we should actually remove the extra comma.
Unless you confirm you really don't want to.
Attachment #586740 - Flags: review?(gavin.sharp)
Comment on attachment 586740 [details] [diff] [review]
(Av1a-FF) Improve code

sure, removing extra commas is fine.
Attachment #586740 - Flags: review?(gavin.sharp)
Comment on attachment 586740 [details] [diff] [review]
(Av1a-FF) Improve code

(In reply to Serge Gautherie (:sgautherie) from comment #3)
> and more tweaks.

(As interdiff is blank :-<)
They are:
*Removes unused 'const Cc'.
*Moves 'const Ci' to were it is used.
*Adds a few linebreaks to reduce some overlong lines.
*Moves/Enhances some comments to their own line.
*Aligns 2 'null,'
*Removes 2 blanks lines
*Enhances a todo() message.
*Removes useless 'lastElement = null'.
Attachment #586740 - Flags: review?
Attachment #586740 - Flags: review? → review?(gavin.sharp)
Comment on attachment 586740 [details] [diff] [review]
(Av1a-FF) Improve code

"
gavin>	most of these changes aren't useful
sgautherie>	k, so be it.
"

Assuming f-.
Attachment #586740 - Attachment is obsolete: true
Attachment #586740 - Flags: review?(gavin.sharp) → feedback-
Comment on attachment 584025 [details] [diff] [review]
(Av1-FF) Improve code
[Checked in: See comment 7]

https://hg.mozilla.org/mozilla-central/rev/cf8c9f9aeefc
Av1-FF, with comment 4 suggestion(s),
and 2 tweaks only to that code.
Attachment #584025 - Attachment description: (Av1-FF) Improve code → (Av1-FF) Improve code [Checked in: See comment 7]
Flags: in-testsuite+
Target Milestone: --- → Firefox 12

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.

Assignee: bugzillamozillaorg_serge_20140323 → nobody
Status: ASSIGNED → NEW
Severity: minor → S4
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: