620319 - crash [@ nsHTMLEditor::GetInlinePropertyBase] if !aAttribute

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[timeless]

998 nsHTMLEditor::GetInlinePropertyBase(nsIAtom *aProperty, 
999                              const nsAString *aAttribute,
1000                              const nsAString *aValue,

1037   if ((NS_SUCCEEDED(result)) && currentItem)
1042     if (isCollapsed)

1047       if (aAttribute)
1048       {
1049         nsString tString(*aAttribute); //MJUDGE SCC NEED HELP

1060       if (isSet) 
1061       {
1062         *aFirst = *aAny = *aAll = theSetting;
1063         return NS_OK;

1065       if (!useCSS) {
1067         IsTextPropertySetByContent(collapsedNode, aProperty, aAttribute, aValue,
1068                                    isSet, getter_AddRefs(resultNode), outValue);

!isSet seems useless given 1063
1071         if (!isSet && aCheckDefaults) 

1076           if (TypeInState::FindPropInList(aProperty, *aAttribute, outValue, mDefaultStyles, index))

Comment 1

[timeless]

ok, so the isSet check is needed because:

1067         IsTextPropertySetByContent(collapsedNode, aProperty, aAttribute, aValue,
1068                                    isSet, getter_AddRefs(resultNode), outValue);

mutates isSet

Comment 2

[timeless]

fwiw, i think i've tried fighting this stuff every couple of years, the code should really have been moved to using nsAString& instead of nsAString*, and it could use Voidable strings or something if it needed to.

From memory, there's one or maybe two paths which actually do deal in null aAttributes, and most other paths don't think they'll send in null pointers. But proving this is really hard.

Comment 3

[(no longer active)]

(In reply to comment #2)
> fwiw, i think i've tried fighting this stuff every couple of years, the code
> should really have been moved to using nsAString& instead of nsAString*, and
> it could use Voidable strings or something if it needed to.

This is a good idea, we should do this.

Comment 4

[Fabien Cazenave [:kaze]]

Attachment: patch proposal

Comment 5

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

http://hg.mozilla.org/mozilla-central/rev/f312978b8b05