Review board's interdiff highlighting misses removed code from patch

RESOLVED FIXED

Status

P1
normal
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: jib, Assigned: smacleod)

Tracking

(Blocks: 1 bug)

Details

(Whiteboard: [to be fixed in core], URL)

See URL.

Expected results:
- dom/media/test/test_streams_srcObject.html showing 37 lines removed.

Actual results:
- dom/media/test/test_streams_srcObject.html showing 9 lines removed.
Summary: Review board's interdiff highlighting misses removed code in this patch → Review board's interdiff highlighting misses removed code in from patch
Summary: Review board's interdiff highlighting misses removed code in from patch → Review board's interdiff highlighting misses removed code from patch

Updated

3 years ago
Assignee: nobody → smacleod
Priority: -- → P1
(Assignee)

Comment 1

3 years ago
I'm almost certain this is a Review Board core bug. The question is whether it has been fixed on the development branches yet. I'll make sure the right people know about this case and see about tracking the fix.

Going to link some relevant info from inspecting this stuff:

Revision 1:
HG Commit: http://reviewboard-hg.mozilla.org/gecko/rev/0315a19a0a19c9f3602467f51659cdeb17bcec0d
HG Parent: http://reviewboard-hg.mozilla.org/gecko/rev/3432f499b158 (Same as Revision 2)
RB Diff: https://reviewboard.mozilla.org/r/13201/diff/1#19
Relevant HG Diff: http://reviewboard-hg.mozilla.org/gecko/rev/0315a19a0a19c9f3602467f51659cdeb17bcec0d#l20.1

Interdiff:
https://reviewboard.mozilla.org/r/13201/diff/1-2#0

Revision 2:
HG Commit: http://reviewboard-hg.mozilla.org/gecko/rev/b1f8b6836982e36e2ddd3a92f9cf805ad0382f5
HG Parent: http://reviewboard-hg.mozilla.org/gecko/rev/3432f499b158 (Same as Revision 1)
RB Diff: https://reviewboard.mozilla.org/r/13201/diff/2#19
Relevant HG Diff: http://reviewboard-hg.mozilla.org/gecko/rev/b1f8b6836982e36e2ddd3a92f9cf805ad0382f5#l20.1
Whiteboard: [to be fixed in core]

Comment 2

3 years ago
Can this be the same issue I'm seeing in bug 1193341 comment 31? The removal of _checkIfURIisSecure from InsecurePasswordUtils.jsm doesn't show up in the interdiff, but it's there.

It appears if you see the entire new patch:

https://reviewboard.mozilla.org/r/18495/diff/5/

It doesn't if you see the individual changes:

https://reviewboard.mozilla.org/r/18495/diff/4/
https://reviewboard.mozilla.org/r/18495/diff/4-5/
Product: Developer Services → MozReview

Updated

3 years ago
Blocks: 1255654
Is this issue fixed?

Comment 4

2 years ago
Seems that the expected results in comment 0 are now actual results: I see 37 lines removed, 58-94.  Not sure if this is just from rebase-filtering being turned off, however.  But for all intents and purposes, it appears fixed right now.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.