To reproduce, go to https://addons.mozilla.org/en-US/editors/review/91162 and try both the code viewer and the diff tool. You'll see that the code viewer expands the chrome JAR file, while the diff tool doesn't. This seems to be a recent and frequent problem, according to the reports I've received.
Another example: https://addons.mozilla.org/en-US/editors/review/86895
OK, this is happening to me about 50% of the time, on top of reports received from other editors. -> major
Severity: normal → major
I've noticed this too and its definitely a recent issue (I'm not sure which release of AMO the regression is from though). I think it should be bumped up the priority queue as potentially dangerous code may be being missed in updates because the jar doesn't show any change. p.s. #521427 - dupe.
I don't want to talk about it.
Assignee: nobody → clouserw
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(this is fixed in production too)
Hmm, I might have lied on this one. Appears to still be intermittent.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Alright, this is caused by Cesar's patch in r56873 and wenzel's patch in 58938. There are a few issues: 1) $isJar in _unwrapDiff() will never be true because _computeArchiveHash() doesn't extract jar files (the by_preg flag doesn't match .jar or .xpi). 2) _computeArchiveHash() is clearning out $v['content'] in an effort to save memory (yay) but _unwrapDiff(), after $isJar matches, is trying to write ['content'] to disk so it can extract it. Cesar/wenzel, do you have time to fix this before monday?
Since I regressed, I will take it.
Assignee: clouserw → a.sacred.line+bugzilla
Status: REOPENED → ASSIGNED
Created attachment 422916 [details] [diff] [review] fix Here is a fix. The problem was closer to your second point. The v['content'] was being destroyed, but we needed it to be present for jar/xpi files so we can extract it to disk. What puzzles me is that this was only occurring, as stated in comment 2, 50% of the time. We do match on $isJar because we join the matched files with the return value of listContents(), which lists all the files in the archive. I made some quick adjustments that I hope is ok. I also made it possible for us to properly expand .JAR files. As well, the view page will now not be double-spaced. Which I'm pretty sure is a bug that has never been filed.
Attachment #422916 - Flags: review?(clouserw)
Comment on attachment 422916 [details] [diff] [review] fix That works for me, please commit. Thanks cesar
Attachment #422916 - Flags: review?(clouserw) → review+
Thanks. In r60986
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago → 9 years ago
Resolution: --- → FIXED
Verified FIXED on https://preview.addons.mozilla.org/en-US/firefox/files/diff/75679/; I could easily reproduce it on production, and it worked fine on staging.
Status: RESOLVED → VERIFIED
Could we push this ASAP? 3.6 is out and we're having a crapload of alleged instal.rdf updates in the review queue. Thanks!
Reclassifying editor bugs and changing to a new whiteboard flag. Spam, spam, spam, spam...
Whiteboard: [required amo-editors] → [ReviewTeam]
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.