Closed Bug 731532 Opened 8 years ago Closed 8 years ago
Fix issues in test pilot validation
Various issues were found in the AMO validation of the test pilot xpi - see attached files. We need to fix these asap so we can start pushing out studies. For the innerHTML stuff, see also https://developer.mozilla.org/en/XUL_School/DOM_Building_and_HTML_Insertion
Based on the discussion linked, I think we should push back on the evalInSandbox warning. https://forums.mozilla.org/addons/viewtopic.php?f=14&t=4532&view=previous&sid=fb4b5f292960eda5d89a2198a8df5dd4
Status: NEW → ASSIGNED
(In reply to Irving Reid (:irving) from comment #1) > Based on the discussion linked, I think we should push back on the > evalInSandbox warning. > > https://forums.mozilla.org/addons/viewtopic. > php?f=14&t=4532&view=previous&sid=fb4b5f292960eda5d89a2198a8df5dd4 Agreed.
Mostly replaced assignments to innerHTML with textContent, except one place where I use the HTML sanitizer service - this needs to be tested.
Comment on attachment 601945 [details] [diff] [review] Work in progress, needs testing This looks fine. I've been using it for the test we've got running at the moment and I've not noticed any issues. We've not tested the survey generator, but we can always hook that up later. The AMO validator output looks a lot better as well. So I think this looks fine, but I'm going to see if Gregg can give us some feedback as well today before we land it.
Comment on attachment 601945 [details] [diff] [review] Work in progress, needs testing Looking back, I think I used appendElement() in a few places where I probably should have used appendChild() to add elements to the HTML content - I'm not sure whether these are interchangeable.
changed appendElement() to appendChild() and fixed the error in addEventListener. Still needs testing, particularly the survey_generator.js changes.
Comment on attachment 603030 [details] [diff] [review] Test pilot cleanup WIP, version 2 Review of attachment 603030 [details] [diff] [review]: ----------------------------------------------------------------- As discussed on irc, I've been running this for the last few days and not found any issues. Although survey-generator.js may need a few more tweaks, we'll be testing any surveys before pushing them out, so I think we're all-right to go with it as-is. Hence r=me.
Attachment #603030 - Flags: feedback?(mbanner) → review+
Checked in: http://hg.mozilla.org/comm-central/rev/66fd8a08a63b I also did a version bump for test pilot so that we can push these to AMO: http://hg.mozilla.org/comm-central/rev/6a007b23cc21
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 13.0
Comment on attachment 603030 [details] [diff] [review] Test pilot cleanup WIP, version 2 [Triage Comment] Taking to beta as I want to get the version of test pilot we're distributing, synced up with the version on AMO.
Attachment #603030 - Flags: approval-comm-beta+
You need to log in before you can comment on or make changes to this bug.