Last Comment Bug 686705 - Source Editor with the textarea fallback displays undefined without a placeholderText
: Source Editor with the textarea fallback displays undefined without a placeho...
Status: RESOLVED FIXED
[fixed-in-fx-team]
:
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 9
Assigned To: Panos Astithas [:past]
:
:
Mentors:
Depends on:
Blocks:
  Show dependency tree / graph
 
Reported: 2011-09-14 10:25 PDT by Panos Astithas [:past]
Modified: 2011-09-21 04:47 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Attachments
Working patch (1.53 KB, patch)
2011-09-14 10:31 PDT, Panos Astithas [:past] 		mihai.sucan: review+
Details | Diff | Splinter Review
[in-fx-team] Patch v2 (1.90 KB, patch)
2011-09-15 09:37 PDT, Panos Astithas [:past] 		no flags Details | Diff | Splinter Review
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Description Panos Astithas [:past] 2011-09-14 10:25:19 PDT 
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] 2011-09-14 10:31:17 PDT 
Created attachment 560182 [details] [diff] [review]
Working patch

Simple fix.
Comment 2 Mihai Sucan [:msucan] 2011-09-15 06:16:33 PDT 
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] 2011-09-15 09:37:42 PDT 
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] 2011-09-16 02:21:23 PDT 
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) 2011-09-21 04:47:44 PDT 
https://hg.mozilla.org/mozilla-central/rev/01ef83aa3f02
Note You need to log in before you can comment on or make changes to this bug.