53673 - multiline textbox does not accept initial value

Status: RESOLVED FIXED

(Core :: XUL, defect, P4)

Description

[Arthur Barrett]

<textfield multiline="true" cols="80" rows="5" id="resstr" value="this initial 
value does not appear (for me) but I dont know why"/>

says it all...

Comment 1

[Chris Waterson]

Could you attach a test case to the bug? Correcting component, -> HTML Form Controls

Comment 2

[Arthur Barrett]

Surely NOT HTML!  XUL.  Will attach a .XUL file to make it clear.

Comment 3

[Arthur Barrett]

Attachment: XUL test set. Just look at the contents of the MULTILINE TEXTFIELD on start, tested with mozilla -chome blah blah if it makes any difference

Comment 4

[Arthur Barrett]

<html:textarea> does not take a value="" attribute, so I thought it was 
definite to XUL.

But yes, <html:textarea>blah</html:textarea> exhibits the same behaviour in a 
XUL file.

started with mozilla -chome chrome://blah

will attach updated test set

Comment 5

[Arthur Barrett]

Attachment: updated test set shows problem with <html:textarea>blah</html:textarea>

Comment 6

[John Morrison]

Yeah, this works correctly for <textarea>foo</textarea> in HTML documents, but
not for <html:textarea>, or the <textarea> XBL binding (which is what 
<textfield multiline="true"> actually maps to in xulBindings.xml). 
It really needs to look at the child text nodes to initially set the .value
property, but updating the .value property seems to work correctly (not 
sure how the binding really works myself). 

-> Ben, the binding meister

Comment 7

[Arthur Barrett]

So if I want to go bug hunting is this the place to start ?

http://lxr.mozilla.org/mozilla/source/xpfe/global/resources/content/xulBindings.
xml#357 

Also I do not understand how 
http://lxr.mozilla.org/mozilla/source/xpfe/global/resources/content/xulBindings.
xml#242 binds textarea to textfield multiline="true" since it is not inherited 
and it is not an implemented property.  Suppose I dont really have to 
understand, but it'd be nice.

Comment 8

[Arthur Barrett]

Attachment: A better (simpler) test

Comment 9

[Arthur Barrett]

Attachment: Oops try again, a simpler test !

Comment 10

[ckritzer (gone)]

XPAPPs & XPToolkit are jrgm's area

Comment 11

[John Morrison]

Sigh ... (ckritzer's been into the rum and egg-nog again).

Comment 12

[ckritzer (gone)]

Okay, not only am I an idiot, I'm an embarrassed idiot.

Sorry guys, I meant to change the QAContact, not Assigned field.

Comment 13

[Fabian Guisset]

Looks like bug 42646. This belongs to hyatt really.

Comment 14

[Fabian Guisset]

the <html:textarea> part of the last testcase works for me on bugzilla, but not
on a local file (wonder why, really)
the <textfield multiline="true"> part of the last testcase never works.
CC'ing jst and rods, because when I use setAttribute("value", ...) and
.value=..., it works just fine. So for some reason the setter fails when first
creating the textfield, but succeeds later on. Strange.

Comment 15

[Eric Hodel]

Attachment: reduced testcase

Comment 16

[Arthur Barrett]

Is it possible to get this a target milestone of 9.x ?

This is an important bug for me since I am developing software which uses 
Mozilla/XUL, I know it doesnt affect the API, but still....

Comment 17

[Brendan Eich [:brendan]]

Dunno if ben has even looked at this.  Cc'ing hyatt, pinging ben.

/be

Comment 18

[John Morrison]

(This is now called a <textbox multiline="true", just to clarify). 

You could have a look at the definition of the XBL for this widget, and 
get it to properly support the 'value' attribute. But, as a workaround,
you can set the value, in an onload handler or otherwise (do you really 
need to statically set the value in the initial xul file?).

Comment 19

[Ben Goodger (use ben at mozilla dot org for email)]

-> hyatt

Comment 20

[Asa Dotzler [:asa]]

Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)

Comment 21

[José Antonio Sánchez Sánchez]

Morrison ->  i.e. if you want to set the value from an RDF DataSource. in this 
case the value is set dinamically in the initial xul file, but not with 
an onload handler.

Comment 22

[Asa Dotzler [:asa]]

*** Bug 122785 has been marked as a duplicate of this bug. ***

Comment 23

[José Antonio Sánchez Sánchez]

for someone who needs this feature. 
You can solve this trouble temporally this way: 

Create a xbl binding that extends from
chrome://global/content/bindings/textbox.xml#textarea;

Constructor of the binding must have this line:

this.value = this.getAttribute("value");

In addition you must define a css style for the binding in order to display it
like a xul textbox with a multiline attribute. 

Comment 24

[[e-mail]]

Attachment: another test case...

Comment 25

[[e-mail]]

Attachment: Proposed patch

Two points:
1. <constructor action="..."> doesn't seem to work
2. this.boxObject.getProperty('value') doesn't seem to work

Comment 26

[jag (Peter Annema)]

<constructor action="..."/> not working may or may not be by design. Ask hyatt.

this.boxObject.getProperty("...") should work (if that property was previously set).

What the old code did (assuming action="..." worked properly) was take the
current value and store it on the boxObject whenever the user switched skins
(when live skin switching was still supported). The XBL object itself would be
destructed, and a new one constructed, while the boxObject would stick around,
so the new XBL object could get the stored value and restore itself.

Since we dropped support for live skin switching, we might be able to just drop
that code, in which case your patch is fine as is if you remove the
<destructor>. If we want to keep support for it in our widgets, you need to do
something like:

<constructor>
  var value = this.boxObject.getProperty("value");
  if (value)
    this.boxObject.removeProperty("value");
  else
    value = this.getAttribute("value");

  if (value)
    this.inputField.value = value;
</constructor>

<destructor>
  var value = this.inputField.value;
  if (value)
    this.boxObject.setProperty("value", value);
</destructor>

Comment 27

[jag (Peter Annema)]

As Neil pointed out, the above needs some kind of check for null vs. "" returned
from getProperty() to detect when a live skin switch is done if the user deleted
the textbox's value. Otherwise we'd pick up the initial value from the attribute.

Comment 28

[[e-mail]]

Attachment: Better patch

Comment 29

[jag (Peter Annema)]

Comment on attachment 107474 [details] [diff] [review]
Better patch

Nice. sr=jag. Could you add a comment about why the empty <children/> is
needed, with a reference to the bug#?

Could you also file bugs on the action="" issue, and a bug on dealing with live
skin switching support on textboxes (either remove it, or fix it)?

Comment 30

[Vedran Miletic]

retargeting

Comment 31

[Benjamin Smedberg]

Comment on attachment 107474 [details] [diff] [review]
Better patch

Neil is a peer now, can we get this checked in please? it seems really
low-risk.

Comment 32

[Brendan Eich [:brendan]]

For 1.6b?

/be

Comment 33

[Brendan Eich [:brendan]]

Er, yeah, for 1.6b!  Who is checking in?

/be

Comment 34

[Benjamin Smedberg]

I got this fixed for seamonkey, asked bryner for approval to land it into the
new toolkit as well.

Comment 35

[Benjamin Smedberg]

Attachment: port patch to new toolkit

Re-attaching patch for firebird review.

Comment 36

[Brian Ryner (not reading)]

Comment on attachment 136862 [details] [diff] [review]
port patch to new toolkit

checked into toolkit/.

Comment 37

[David Leverton]

This seems to have caused textareas to contain whitespace by default, for
example,  when creating a new folder in Bookmark Manager the "Description" field
contains spaces. (in Moz 2004011508)

Comment 38

[[e-mail]]

Attachment: Fix regression caused by xbl content sink change

The XBL content sink now honours whitespace inside HTML tags, so we need to
close up the child tags.

Comment 39

[David Hyatt]

Comment on attachment 139289 [details] [diff] [review]
Fix regression caused by xbl content sink change

sr=hyatt