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)
Firefox
General
Tracking
()
NEW
Firefox 12
People
(Reporter: sgautherie, Unassigned)
References
()
Details
Attachments
(1 file, 1 obsolete file)
10.40 KB,
patch
|
Gavin
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Comment 1•12 years ago
|
||
Attachment #584025 -
Flags: review?(gavin.sharp)
Comment 2•12 years ago
|
||
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+
Reporter | ||
Comment 3•12 years ago
|
||
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 4•12 years ago
|
||
Comment on attachment 586740 [details] [diff] [review] (Av1a-FF) Improve code sure, removing extra commas is fine.
Attachment #586740 -
Flags: review?(gavin.sharp)
Reporter | ||
Comment 5•12 years ago
|
||
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?
Reporter | ||
Updated•12 years ago
|
Attachment #586740 -
Flags: review? → review?(gavin.sharp)
Reporter | ||
Comment 6•12 years ago
|
||
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-
Reporter | ||
Comment 7•12 years ago
|
||
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]
Reporter | ||
Updated•12 years ago
|
Flags: in-testsuite+
Target Milestone: --- → Firefox 12
Comment 8•2 years ago
|
||
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
Updated•2 years ago
|
Severity: minor → S4
You need to log in
before you can comment on or make changes to this bug.
Description
•