If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Diff bar misses changes at the bottom of the file

RESOLVED FIXED in Q3 2011

Status

addons.mozilla.org Graveyard
Admin/Editor Tools
P3
major
RESOLVED FIXED
6 years ago
2 years ago

People

(Reporter: eviljeff, Assigned: kmag)

Tracking

unspecified
Q3 2011

Details

(Whiteboard: [ReviewTeam], URL)

Attachments

(3 attachments)

(Reporter)

Description

6 years ago
In the above diff there are changes added to end of overlay.js which aren't shown in the diff bar (its clear rather than green).  Other changes in the file are indicated in green.

If there are no other changes near the bottom of the existing code then we might be missing potentially dangerous new code.

(unfortunately I've had to reject the addon in the url so the link won't work until the xpi is restored - its not working for me right now)
Priority: -- → P3
Whiteboard: [required amo-editors]
Target Milestone: --- → 6.1.8
Kris, is this in the code that you wrote?
Target Milestone: 6.1.8 → Q3 2011
I think it's in a section with both Andy's code and mine, but I'll take it. I think jQuery's throwing an exception that the event listener is eating, because something strange is happening in the generated DOM. The last diff marker winds up with an empty style attribute after passing a non-empty CSS object to .attr().

I'm going to need to find a way to reproduce this locally. Can someone get me a copy of the two files in the linked diff URL?
Assignee: nobody → maglione.k
-> major, since this can lead to unreviewed code.
Severity: normal → major
(Reporter)

Comment 4

6 years ago
Created attachment 547475 [details]
Mar_Mod-1.1
(Reporter)

Comment 5

6 years ago
Created attachment 547476 [details]
Mar_Mod-1.0
Created attachment 547508 [details] [diff] [review]
Patch

The height of the last marker was turning up NaN. It looks like that line didn't survive my jQuery-ification. It originally used .getBoundingClientRect(), but .offset() obviously doesn't have a .bottom property.
Attachment #547508 - Flags: review?(amckay)

Comment 7

6 years ago
Comment on attachment 547508 [details] [diff] [review]
Patch

Looks good, thanks Kris.
Attachment #547508 - Flags: review?(amckay) → review+

Updated

6 years ago
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Comment 8

6 years ago
http://github.com/jbalogh/zamboni/commit/aa231d
Reclassifying editor bugs and changing to a new whiteboard flag. Spam, spam, spam, spam...
Whiteboard: [required amo-editors] → [ReviewTeam]
https://hg.mozilla.org/integration/mozilla-inbound/rev/62207930924a
https://hg.mozilla.org/integration/mozilla-inbound/rev/6931d5b6f055
https://hg.mozilla.org/integration/mozilla-inbound/rev/31cdc85ce16e
https://hg.mozilla.org/integration/mozilla-inbound/rev/0636839be2d1
Whoops. Added wrong bug number
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.