Closed Bug 506649 Opened 11 years ago Closed 11 years ago

nsFocusManager::GetSelectionLocation has unreachable code block due to shadowed local variable

Categories

(Core :: DOM: Core & HTML, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
status1.9.2 --- beta1-fixed

People

(Reporter: timeless, Assigned: timeless)

References

(Blocks 1 open bug, )

Details

(Keywords: coverity)

Attachments

(1 file)

1846 nsFocusManager::GetSelectionLocation(nsIDocument* aDocument,
1940 nsIFrame *newCaretFrame = nsnull;
1943           do {
1947             nsIFrame* newCaretFrame = static_cast<nsIFrame*>(frameTraversal->CurrentItem());
1951           } while (!newCaretContent || newCaretContent == startContent);
1953 if (newCaretFrame && newCaretContent) { 1954 // If the caret is exactly at the same position of the new frame, 1955 // then we can use the newCaretFrame and newCaretContent for our position 1956 nsRefPtr<nsCaret> caret; 1957 aPresShell->GetCaret(getter_AddRefs(caret)); 1958 nsRect caretRect; 1959 nsIView *caretView; 1960 caret->GetCaretCoordinates(nsCaret::eClosestViewCoordinates, 1961 domSelection, &caretRect, 1962 &isCollapsed, &caretView); 1963 nsPoint framePt; 1964 nsIView *frameClosestView = newCaretFrame->GetClosestView(&framePt); 1965 if (caretView == frameClosestView && caretRect.y == framePt.y && 1966 caretRect.x == framePt.x) { 1967 // The caret is at the start of the new element. 1968 startFrame = newCaretFrame; 1969 startContent = newCaretContent; 1970 if (endOfSelectionInStartNode) { 1971 endContent = newCaretContent; // Ensure end of selection is not before start 1972 } 1973 } 1974 } 

I've intentionally let the last blob collapse, it's unreachable :)
Keywords: coverity
This is a regression, though, I'm not yet sure what problems this cause.
When fixing this, it clearly needs also testcases.
Flags: blocking1.9.2?
i'm not in a position to write tests for this problem. this is the "obvious" patch.
Attachment #390919 - Flags: review+
Attachment #390919 - Flags: review?(enndeakin)
Comment on attachment 390919 [details] [diff] [review]
the obvious patch

Even though this is a trivial patch, could you enn check it.
Comment on attachment 390919 [details] [diff] [review]
the obvious patch

Looks OK. Also, the assignment startFrame = newCaretFrame isn't needed.
Attachment #390919 - Flags: review?(enndeakin) → review+
Assignee: nobody → timeless
http://hg.mozilla.org/mozilla-central/rev/1b368a826f91
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Flags: wanted1.9.2+
Flags: blocking1.9.2?
Flags: blocking1.9.2-
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.