686705 - Source Editor with the textarea fallback displays undefined without a placeholderText

Status: RESOLVED FIXED

Description

[Panos Astithas [:past] (please needinfo?)]

When creating a SourceEditor without a placeholderText in the configuration, we get the string "undefined" when using a textarea. Also a warning is displayed in case strict mode is enabled:

JavaScript strict warning: resource:///modules/source-editor-textarea.jsm, line 132: reference to undefined property aConfig.placeholderText

Comment 1

[Panos Astithas [:past] (please needinfo?)]

Created attachment 560182 [details] [diff] [review]
Working patch

Simple fix.

Comment 2

[Mihai Sucan [:msucan]]

Comment on attachment 560182 [details] [diff] [review]
Working patch

Review of attachment 560182 [details] [diff] [review]:
-----------------------------------------------------------------

Hah, I missed this when I did my patches. Thanks for the fix!

::: browser/devtools/sourceeditor/source-editor-textarea.jsm
@@ +129,5 @@
>  
>      this._textbox.style.MozTabSize = this._tabSize;
>  
> +    this._textbox.setAttribute("value", aConfig.placeholderText ?
> +                               aConfig.placeholderText : "");

Perhaps it would be more concise to do:

this._textbox.setAttribute("value", aConfig.placeholderText || "");

Comment 3

[Panos Astithas [:past] (please needinfo?)]

Created attachment 560363 [details] [diff] [review]
[in-fx-team] Patch v2

(In reply to Mihai Sucan [:msucan] from comment #2)
> Comment on attachment 560182 [details] [diff] [review]
> Working patch
> 
> Review of attachment 560182 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hah, I missed this when I did my patches. Thanks for the fix!
> 
> ::: browser/devtools/sourceeditor/source-editor-textarea.jsm
> @@ +129,5 @@
> >  
> >      this._textbox.style.MozTabSize = this._tabSize;
> >  
> > +    this._textbox.setAttribute("value", aConfig.placeholderText ?
> > +                               aConfig.placeholderText : "");
> 
> Perhaps it would be more concise to do:
> 
> this._textbox.setAttribute("value", aConfig.placeholderText || "");

Absolutely.

Comment 4

[Mihai Sucan [:msucan]]

Comment on attachment 560363 [details] [diff] [review]
[in-fx-team] Patch v2

Pushed:
https://hg.mozilla.org/integration/fx-team/rev/01ef83aa3f02

Comment 5

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/mozilla-central/rev/01ef83aa3f02