Closed
Bug 726287
Opened 12 years ago
Closed 12 years ago
misc decomtamination in nsHypertextAccessible
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla13
People
(Reporter: tbsaunde, Assigned: tbsaunde)
References
Details
Attachments
(1 file, 1 obsolete file)
8.68 KB,
patch
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•12 years ago
|
||
There's plenty more to do here, but I think this is an improvement (atleast I do at 0600)
Attachment #596296 -
Flags: review?(surkov.alexander)
Comment 2•12 years ago
|
||
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
Attachment #596296 -
Flags: review?(surkov.alexander) → review+
Comment 3•12 years ago
|
||
Trevor, it'd be great if you get a chance to finish and land it.
Assignee | ||
Comment 4•12 years ago
|
||
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?
Attachment #596296 -
Attachment is obsolete: true
Comment 6•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/aa7d28b45ede
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in
before you can comment on or make changes to this bug.
Description
•