726287 - misc decomtamination in nsHypertextAccessible

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Comment 1

[Trevor Saunders (:tbsaunde)]

Attachment: start

There's plenty more to do here, but I think this is an improvement (atleast I do at 0600)

Comment 2

[alexander :surkov (:asurkov)]

Comment on attachment 596296 [details] [diff] [review]
start

Review of attachment 596296 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/html/nsHyperTextAccessible.cpp
@@ +1777,5 @@
>  
>    // Remove collapsed ranges
>    PRUint32 numRanges = aRanges->Length();
>    for (PRUint32 count = 0; count < numRanges; count ++) {
> +    if ((*aRanges)[count]->Collapsed()) {

while you are here, could you rename count to idx please?

@@ +1829,4 @@
>  
>    // Get end point
> +  nsINode* endNode = range->GetEndParent();
> +  PRInt32 endOffset = range->EndOffset();

you can keep these under common comment "Get start and end points"

@@ +1833,2 @@
>  
> +  // Make sure start is before end, by swapping offsets

by swapping DOM points, dot in the end

@@ +1833,3 @@
>  
> +  // Make sure start is before end, by swapping offsets
> +  // This occurs when the user selects backwards in the text

keep this on the same line as prev comment, dot in the end

@@ +2342,5 @@
>    for (PRUint32 index = 0; index < rangeCount; index++) {
>      nsRange* range = ranges[index];
>  
>      PRInt16 result;
> +    nsresult rv = range->ComparePoint(DOMNode, aNodeOffset, &result);

you could add dexcpomed version of ComparePoint to nsRange, up to you

@@ +2350,5 @@
>      // See also bug 460690.
>      if (result == 0) {
> +      nsINode* endNode = range->GetEndParent();
> +      PRInt32 endOffset = range->EndOffset();
> +      if (aNode == endNode && aNodeOffset == endOffset)

sounds like you don't need local variables for those

Comment 3

[alexander :surkov (:asurkov)]

Trevor, it'd be great if you get a chance to finish and land it.

Comment 4

[Trevor Saunders (:tbsaunde)]

Attachment: patch 2

aAlex, iirc this fixes all of your nits accept the qi to nsIDOMNode for a nsRange method which I think I'm for leaving be for now.  I really doubt I'll have time to deal with try etc to land this before friday, so if you want this done before then would you mind landing it?

Comment 5

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/aa7d28b45ede

Comment 6

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/aa7d28b45ede