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)

x86
Windows 7
defect
Not set
normal

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.
Assignee: nobody → fayearthur+bugs
Assignee: fayearthur+bugs → nobody
Depends on: 661408
Depends on: 661588
We've fixed these issues in seperate bugs.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.