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

Status: NEW

(Firefox :: General, defect)

Comment 1

[Serge Gautherie (:sgautherie)]

Attachment: (Av1-FF) Improve code [Checked in: See comment 7]

Comment 2

[:Gavin Sharp [email: [e-mail]]]

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.

Comment 3

[Serge Gautherie (:sgautherie)]

Attachment: (Av1a-FF) Improve code

Av1-FF, with comment 2 suggestion(s),
and more tweaks.


(In reply to Gavin Sharp (use [e-mail] 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.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 586740 [details] [diff] [review]
(Av1a-FF) Improve code

sure, removing extra commas is fine.

Comment 5

[Serge Gautherie (:sgautherie)]

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'.

Comment 6

[Serge Gautherie (:sgautherie)]

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-.

Comment 7

[Serge Gautherie (:sgautherie)]

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.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

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