672956 - Diff bar misses changes at the bottom of the file

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Admin/Editor Tools, defect, P3)

Description

[Andrew Williamson [:eviljeff]]

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)

Comment 1

[Wil Clouser [:clouserw]]

Kris, is this in the code that you wrote?

Comment 2

[Kris Maglione [:kmag]]

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?

Comment 3

[Jorge Villalobos [:jorgev] (he/him)]

-> major, since this can lead to unreviewed code.

Comment 4

[Andrew Williamson [:eviljeff]]

Attachment: Mar_Mod-1.1

Comment 5

[Andrew Williamson [:eviljeff]]

Attachment: Mar_Mod-1.0

Comment 6

[Kris Maglione [:kmag]]

Attachment: 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.

Comment 7

[Andy McKay]

Comment on attachment 547508 [details] [diff] [review]
Patch

Looks good, thanks Kris.

Comment 8

[Andy McKay]

http://github.com/jbalogh/zamboni/commit/aa231d

Comment 9

[Jorge Villalobos [:jorgev] (he/him)]

Reclassifying editor bugs and changing to a new whiteboard flag. Spam, spam, spam, spam...

Comment 10

[Wesley Johnston (:wesj)]

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

Comment 11

[Wesley Johnston (:wesj)]

Whoops. Added wrong bug number