Closed
Bug 652990
Opened 13 years ago
Closed 13 years ago
Mozmill 1.5.3 addon review coments need to be addressed
Categories
(Testing Graveyard :: Mozmill, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: cmtalbert, Unassigned)
References
Details
(Whiteboard: [mozmill-1.5.4+])
The 1.5.3 addon review came in. Thanks to the AMO editors! I'm going to paste their comments first, then recommend some options in another comment. We need to do the following: Comments: This version didn't pass full review because of the following issues which the last editor requested be addressed in the next version: 1) You define the non-namespaced variable 'documentLoaded' in the global namespace, which could cause compatibility problems with other add-ons. 2) You set the following preference defaults which have serious performance implications and don't seem to be required or suggested by your add-on. If you choose to continue setting them, please make it clear in your description that you do so and that they do have serious performance implications, and either explain how to reset them to their former values or provide an option to do so. pref("nglayout.debug.disable_xul_cache", true); pref("nglayout.debug.disable_xul_fastload", true); 3) You define a DOM node with the inadequately namespaced id 'open-mm' in your browser overlay, which could cause compatibility problems with other add-ons. 4) As your XPI contains JAR files, its chrome.manifest should ideally specify em:unpack.
(In reply to comment #0) > > We need to do the following: > > 1) You define the non-namespaced variable 'documentLoaded' in the global > namespace, which could cause compatibility problems with other add-ons. we need to fix this. > > 2) You set the following preference defaults which have serious performance > implications and don't seem to be required or suggested by your add-on. If you > choose to continue setting them, please make it clear in your description that > you do so and that they do have serious performance implications, and either > explain how to reset them to their former values or provide an option to do so. > > pref("nglayout.debug.disable_xul_cache", true); > pref("nglayout.debug.disable_xul_fastload", true); > Why are we setting these? I think they should be entirely removed. > 3) You define a DOM node with the inadequately namespaced id 'open-mm' in your > browser overlay, which could cause compatibility problems with other add-ons. That's kind of a stretch, but also trivial to fix. > > 4) As your XPI contains JAR files, its chrome.manifest should ideally specify > em:unpack. We should fix this.
Updated•13 years ago
|
Assignee: nobody → fayearthur+bugs
Comment 2•13 years ago
|
||
replaced id's: https://github.com/mozautomation/mozmill/commit/34a035bdb5512048e5448b18e9f76b5b92641a09 https://github.com/mozautomation/mozmill/commit/066dd9e49e138ae451499189965c3008accc77fd
Comment 3•13 years ago
|
||
em:unpack in install.rdf: https://github.com/mozautomation/mozmill/commit/84f3995bc00ef44bcb85c21e65bd19594c04a163 https://github.com/mozautomation/mozmill/commit/6e5963ec98919bfb876a70010558436a9a5e65b0
Comment 4•13 years ago
|
||
We've fixed these issues in seperate bugs.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•8 years ago
|
Product: Testing → Testing Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•