Closed
Bug 581475
Opened 14 years ago
Closed 14 years ago
nsContextMenu cleanup
Categories
(Firefox :: Menus, defect)
Firefox
Menus
Tracking
()
VERIFIED
FIXED
Firefox 4.0b3
People
(Reporter: dao, Assigned: dao)
References
Details
Attachments
(2 files)
(deleted),
patch
|
asaf
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
benjamin
:
approval2.0+
|
Details | Diff | Splinter Review |
- bails out early if the menu isn't going to be displayed
- removes the unused aXulMenu argument
- removes the duplicate property initialization
I've pushed this and the patch in bug 425575 to the tryserver, tests pass.
Attachment #459855 -
Flags: review?(mano)
Comment 1•14 years ago
|
||
Comment on attachment 459855 [details] [diff] [review]
patch
r=mano
Attachment #459855 -
Flags: review?(mano) → review+
Assignee | ||
Updated•14 years ago
|
Attachment #459855 -
Flags: approval2.0?
Comment 2•14 years ago
|
||
Comment on attachment 459855 [details] [diff] [review]
patch
Is nsContextMenu an API that extensions might be using?
Assignee | ||
Comment 3•14 years ago
|
||
Yes, extensions might. Should the menu parameter be kept?
Comment 4•14 years ago
|
||
Maybe: our mozilla2.0 API rules (draft at https://developer.mozilla.org/En/Developer_Guide/Interface_Compatibility#JavaScript.2fXUL_Interfaces)
If retaining compatibility is simply keeping the extra unused parameter, we probably should.
Assignee | ||
Comment 5•14 years ago
|
||
Attachment #461262 -
Flags: approval2.0?
Assignee | ||
Updated•14 years ago
|
Attachment #459855 -
Flags: approval2.0?
Updated•14 years ago
|
Attachment #461262 -
Flags: approval2.0? → approval2.0+
Assignee | ||
Comment 6•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b3
Comment 7•14 years ago
|
||
Looks like all existing tests pass with this change. Marking as verified fixed.
Status: RESOLVED → VERIFIED
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•