Show compatibility warning if a variable called "top" is used in chrome code

RESOLVED FIXED in 6.0.12

Status

addons.mozilla.org Graveyard
Developer Pages
P2
normal
RESOLVED FIXED
7 years ago
2 years ago

People

(Reporter: jorgev, Assigned: basta)

Tracking

unspecified
6.0.12

Details

(Whiteboard: [fx6][post-freeze+])

(Reporter)

Description

7 years ago
As explained in bug 654137, window.top is now read-only. This means that if you have an undefined variable called "top", your add-on will break. Ideally this check should only detect when you try to redefine window.top in some way. Otherwise we should at least check for all assignments done to a variable named "top" in chrome code.

This is only a warning. It should appear in the compatibility message sent to authors, but should not prevent a compatibility bump from happening.
Whiteboard: [fx6] → [fx6][post-freeze+]
Target Milestone: 6.1.0 → 6.0.12
(Reporter)

Comment 1

7 years ago
Message:

The "top" global variable is reserved and cannot be assigned any values starting with Firefox 6. Review your code for any uses of the "top" global, and refer to <LINKED_BUG> for more information.

Krupa:

Depending on how this is implemented, the flag may show up for All-In-One Gestures:
https://addons.mozilla.org/en-US/firefox/addon/all-in-one-gestures/

If the implementation picks up all uses of a variable called "top", this add-on (and a million others) will be flagged. If it looks for assignments only to undeclared "top" variables, then there could be no matches at all.
(Assignee)

Comment 2

7 years ago
Does this apply to JS modules?
(Reporter)

Comment 4

7 years ago
(In reply to comment #2)
> Does this apply to JS modules?

No, it would be best to discard JS modules if possible.
(In reply to comment #0)
> This is only a warning. It should appear in the compatibility message sent
> to authors, but should not prevent a compatibility bump from happening.

As of bug 658161 we no longer show warnings on the compatibility results page.  An author will only be notified when there are compatibility errors, so it wouldn't make sense to show warnings.  

We need to fix something.  We can either make this an error (the easy fix) or start to display compatibility warnings and figure out some way to also notify authors of warnings (the hard fix)
(Reporter)

Comment 6

7 years ago
Wasn't bug 658161 about removing messages that are unrelated to the compatibility push?

Making this and other warnings into errors would produce a very large number of false positives, which is not something we want.
bug 661259 and bug 661261 are required to integrate this new compatibility warning.
(Reporter)

Comment 8

7 years ago
Bug 659681 and bug 660349 are also warnings.
No longer depends on: 661261
(Assignee)

Comment 9

7 years ago
Merged:

https://github.com/mozilla/amo-validator/commit/7778eec40ff8923f8f515d17b702b17690706a8a
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.