58028 - compareBoundaryPoints returns opposite value

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect, P1)

Description

[Dylan Schiemann]

From Bugzilla Helper:
BuildID:    2000100908

The comparison of boundary points in DOM Level 2 Range returns the opposite 
value as listed in the spec.  -1 in place of 1 and vice versa.

Reproducible: Always
Steps to Reproduce:
1. Run testcase

Actual Results:  compareBoundaryPoints should have returned 1 for all four 
scenarios, but returnes -1 instead.  I believe the reference is 
backwards.                     

Expected Results:  return 1 for all four cases                     

Testcase to follow               

Spec statement:

2.5. Comparing Range Boundary-Points
It is possible to compare two Ranges by comparing their boundary-points: 

  short compareBoundaryPoints(in CompareHow how, in Range sourceRange) raises
(RangeException);

where CompareHow is one of four values: START_TO_START, START_TO_END, 
END_TO_END and END_TO_START. The return value is -1, 0 or 1 depending on 
whether the corresponding boundary-point of the Range is before, equal to, or 
after the corresponding boundary-point of sourceRange. 

I believe it is giving the sourceRange relative to the range, rather than the 
range relative tot eh sourceRange.

Comment 1

[Dylan Schiemann]

Attachment: compareBoundaryPoints testcase

Comment 2

[Johnny Stenback (:jst)]

Confirming and reassigning, this needs to be fixed ASAP in mozilla, setting to
milestone mozilla0.9.

Comment 3

[anthonyd]

setting to p1
anthonyd

Comment 4

[anthonyd]

attaching new test case.  much conferring with kin.  START_TO_END and 
END_TO_START are returning the opposite values.  adding kin a jfrancis to cc 
list.

anthonyd

Comment 5

[anthonyd]

Attachment: new test case

Comment 6

[kinmoz]

Just to be clear, it seems that our interpretation of the constants 
START_TO_START, START_TO_END, END_TO_START, and END_TO_END was different from 
what the spec now says.

For example, according to the spec, it says that START_TO_END refers to the 
start of the source range, and the end of the boundary range, but our 
implementation is actually comparing the start of the boundary range against the 
end of the source range.

Fixing this is a simple matter of fixing the switch statement in 
nsRange::CompareBoundaryPoints(), but we also need to make sure the editor code 
that uses CompareBoundaryPoints() is also corrected.

Comment 7

[kinmoz]

By the way the definition of the *_TO_* constants

http://www.w3.org/TR/2000/REC-DOM-Level-2-Traversal-Range-20001113/ranges.html

seems backwards to me, especially if you were to think of them as saying: 
"Compare *_TO_*"

Comment 8

[anthonyd]

Attachment: new test case with correct wording

Comment 9

[anthonyd]

actually, after re-reading what kin and I talked about for this bug, the switch 
cases are all fine, just the return values were incorrect, and the looks like 
there was a small problem with that.  I re-wrote the code to make it a bit more 
clear what should be happening, and I changed the corresponding code in 
htmlrules.  I will attach a patch for kin and joe (and anyone else for that 
matter) to look at.

anthonyd

Comment 10

[anthonyd]

Attachment: patch with changes to compare boundary, rules changes

Comment 11

[anthonyd]

need reviewers and approval.

anthonyd

Comment 12

[anthonyd]

Attachment: new patch with kin's changes

Comment 13

[anthonyd]

can i get some r= and a= please?

anthonyd

Comment 14

[Johnny Stenback (:jst)]

r=jst

Comment 15

[kinmoz]

r,a=[e-mail] (a= assuming that the Editor group now owns range code?)

Comment 16

[anthonyd]

fix is checked in.

anthonyd

Comment 17

[rubydoo123]

catching up on verifications