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

Status: RESOLVED FIXED

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

Description

[timeless]

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 :)

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

This is a regression, though, I'm not yet sure what problems this cause.
When fixing this, it clearly needs also testcases.

Comment 2

[timeless]

Attachment: the obvious patch

i'm not in a position to write tests for this problem. this is the "obvious" patch.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Ok, this is some kind of copy-paste error.
http://mxr.mozilla.org/mozilla1.9.1/source/content/events/src/nsEventStateManager.cpp?mark=5720-5720#5695

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 390919 [details] [diff] [review]
the obvious patch

Even though this is a trivial patch, could you enn check it.

Comment 5

[Neil Deakin]

Comment on attachment 390919 [details] [diff] [review]
the obvious patch

Looks OK. Also, the assignment startFrame = newCaretFrame isn't needed.

Comment 6

[timeless]

http://hg.mozilla.org/mozilla-central/rev/1b368a826f91