Closed Bug 686705 Opened 13 years ago Closed 13 years ago

Source Editor with the textarea fallback displays undefined without a placeholderText

Categories

(DevTools :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 9

People

(Reporter: past, Assigned: past)

Details

(Whiteboard: [fixed-in-fx-team])

Attachments

(1 file, 1 obsolete file)

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
Assignee: nobody → past
Status: NEW → ASSIGNED
Attached patch Working patch (obsolete) — Splinter Review
Simple fix.
Attachment #560182 - Flags: review?(mihai.sucan)
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 || "");
Attachment #560182 - Flags: review?(mihai.sucan) → review+
(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.
Attachment #560182 - Attachment is obsolete: true
Whiteboard: [land-in-fx-team]
Comment on attachment 560363 [details] [diff] [review]
[in-fx-team] Patch v2

Pushed:
https://hg.mozilla.org/integration/fx-team/rev/01ef83aa3f02
Attachment #560363 - Attachment description: Patch v2 → [in-fx-team] Patch v2
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/01ef83aa3f02
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 9
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: