353061 - Uninitialized value used in nsTypedSelection::AddRange()

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect, P3)

Description

[x22RWEN]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060917 Minefield/3.0a1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060917 Minefield/3.0a1

mType is uninitialized when it's used in this comparison:

nsSelection.cpp:
5745  if (mType == nsISelectionController::SELECTION_NORMAL)

Breakpoint 4, nsTypedSelection::AddRange (this=0x90e2878, aRange=0x8ed4ed8)
    at /home/s/cvs/mozilla/layout/generic/nsSelection.cpp:5745
5745      if (mType == nsISelectionController::SELECTION_NORMAL)
(gdb) p mType
$18 = 9096

9096 is not a valid SelectionType :).

Reproducible: Always

Steps to Reproduce:
1. Navigate to http://www.gamefaqs.com
2. Ctrl + a



==9521== Conditional jump or move depends on uninitialised value(s)
==9521==    at 0x6B55D51: nsTypedSelection::AddRange(nsIDOMRange*) (nsSelection.cpp:5745)
==9521==    by 0x6D1211A: nsHTMLCopyEncoder::SetSelection(nsISelection*) (nsDocumentEncoder.cpp:1183)
==9521==    by 0x6CE7A76: nsCopySupport::HTMLCopy(nsISelection*, nsIDocument*, short) (nsCopySupport.cpp:117)
==9521==    by 0x6B4B6AC: nsAutoCopyListener::NotifySelectionChanged(nsIDOMDocument*, nsISelection*, short) (nsSelection.cpp:7657)
==9521==    by 0x6B4EBA9: nsTypedSelection::NotifySelectionListeners() (nsSelection.cpp:7481)
==9521==    by 0x6B4EC0F: nsFrameSelection::NotifySelectionListeners(short) (nsSelection.cpp:2828)
==9521==    by 0x6B557FB: nsTypedSelection::Extend(nsIDOMNode*, int) (nsSelection.cpp:6688)
==9521==    by 0x6B48610: nsTypedSelection::SelectAllChildren(nsIDOMNode*) (nsSelection.cpp:6728)
==9521==    by 0x6A73D02: DocumentViewerImpl::SelectAll() (nsDocumentViewer.cpp:2510)
==9521==    by 0x6F66E77: nsClipboardSelectAllNoneCommands::DoClipboardCommand(char const*, nsIContentViewerEdit*, nsICommandParams*) (nsGlobalWindowCommands.cpp:634)
==9521==    by 0x6F65C62: nsClipboardBaseCommand::DoCommand(char const*, nsISupports*) (nsGlobalWindowCommands.cpp:459)
==9521==    by 0x63BAFC1: nsControllerCommandTable::DoCommand(char const*, nsISupports*) (nsControllerCommandTable.cpp:191)
==9521== 
==9521== Conditional jump or move depends on uninitialised value(s)
==9521==    at 0x6B55D51: nsTypedSelection::AddRange(nsIDOMRange*) (nsSelection.cpp:5745)
==9521==    by 0x6D1211A: nsHTMLCopyEncoder::SetSelection(nsISelection*) (nsDocumentEncoder.cpp:1183)
==9521==    by 0x6CE7E93: nsCopySupport::HTMLCopy(nsISelection*, nsIDocument*, short) (nsCopySupport.cpp:154)
==9521==    by 0x6B4B6AC: nsAutoCopyListener::NotifySelectionChanged(nsIDOMDocument*, nsISelection*, short) (nsSelection.cpp:7657)
==9521==    by 0x6B4EBA9: nsTypedSelection::NotifySelectionListeners() (nsSelection.cpp:7481)
==9521==    by 0x6B4EC0F: nsFrameSelection::NotifySelectionListeners(short) (nsSelection.cpp:2828)
==9521==    by 0x6B557FB: nsTypedSelection::Extend(nsIDOMNode*, int) (nsSelection.cpp:6688)
==9521==    by 0x6B48610: nsTypedSelection::SelectAllChildren(nsIDOMNode*) (nsSelection.cpp:6728)
==9521==    by 0x6A73D02: DocumentViewerImpl::SelectAll() (nsDocumentViewer.cpp:2510)
==9521==    by 0x6F66E77: nsClipboardSelectAllNoneCommands::DoClipboardCommand(char const*, nsIContentViewerEdit*, nsICommandParams*) (nsGlobalWindowCommands.cpp:634)
==9521==    by 0x6F65C62: nsClipboardBaseCommand::DoCommand(char const*, nsISupports*) (nsGlobalWindowCommands.cpp:459)
==9521==    by 0x63BAFC1: nsControllerCommandTable::DoCommand(char const*, nsISupports*) (nsControllerCommandTable.cpp:191)

Comment 1

[Andrew Schultz]

The code is from bug 334256

Comment 2

[Uri Bernstein (Google)]

(In reply to comment #1)
> The code is from bug 334256

Actually, it's from bug 338315. Still my fault, though.

Comment 3

[Uri Bernstein (Google)]

Well, not really my fault...

The problem is here:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/base/src/nsDocumentEncoder.cpp&rev=1.117&mark=1150-1152#1150
The cloning as implemented in nsHTMLCopyEncoder::SetSelection() doesn't copy aSelection's mType into mSelection. It's not easy to add that, either, because mType is a member of nsTypedSelection, which is not exposed in nsISelection.

So, I guess we actually need to create an nsISelection::Clone()? Or does anybody have a better idea?

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Creating Clone() sounds good to me.

Comment 5

[timeless]

http://timeless.justdave.net/mxr-test/garage/source/browser/mozilla/trunk/microb-engine/microb-engine/debian/patches/valgrind/bug66825.diff

is the patch that we have in our tree...

Comment 7

[Mike Schroepfer]

Roc suggested owner?

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Matthew, this is probably easier than the other crappy bugs we've been giving him

Comment 9

[Jesse Ruderman]

Mats has a patch in bug 417895.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Should be fixed now.