43208 - readonly text fields are not readonly

Status: VERIFIED FIXED

(Core :: Layout, defect, P3)

Description

[Stephen P. Morse]

Collect at least one cookie from somewhere

Go to cookie viewer.  The cookie should be displayed (due to bug 43201 you won't 
see the cookies until you force a redraw by covering with another window and 
then uncovering).

Click on one of the cookies.  Its properties should now be displayed.  They 
aren't.  Even forcing a redraw does not bring up the display of the 
properties.

(This has been broken out of bug 43126 by request.)

Comment 1

[Jay Patel [:jay]]

Putting on [nsbeta2+] radar for beta2 fix. 

Comment 2

[Stephen P. Morse]

By removing <div>s and changing <fieldset> to <titledbox> as suggested by hyatt, 
the properties are now being displayed but not properly.  Here are the problems 
with the display:

- it takes a relatively long time for display to appear (it used to be 
intantaneoulsy as soon as you selected the cookie

- textfields of each property have "read only" attributes but they can be 
written into

- changing the selected cookie will cause the property textfields to flicker 
between white and grayed-out and the final color that they wind up in is almost 
random (I haven't figured out any pattern yet). 

Comment 3

[David Hyatt]

These are all textfield glitches.  The tree is now more or less ok.  You should 
probably file individual bugs against the owner of the textfield now.

Closing this one out.

Comment 4

[Stephen P. Morse]

Who is the owner of the textfield?

Comment 5

[Stephen P. Morse]

This bug is *not* fixed.  Will reassig to layout component.  If that's not the 
right owner, please tell me who it.

Comment 6

[Stephen P. Morse]

Reassigning

Comment 7

[harishd]

Triaging Clayton's list:
---------------------------

Eric, could you please take a look at this? I'm not sure who owns "textfield" 
bugs.

Comment 8

[Eric Pollmann]

The bug with textfields is this:
 <input type=text readonly>

This is broken, but setting the readOnly attribute via the DOM works fine.  This 
is a new bug with Ender Lite.  Forwarding over to Mike.

In the old GfxTextControlFrame, the logic to do this is in:
nsGfxTextControlFrame::InitializeTextControl

Particularly, see the section after this line:
  // set readonly and disabled states

nsGfxTextControlFrame::InitializeTextControl was called by 
nsGfxTextControlFrame::InstallEditor.

The InstallEditor logic looks like it migrated into:
nsGfxTextControlFrame2::CreateAnonymousContent, so after the editor is 
initialized here, it should have it's readonly state set (and disabled and 
wrap too, probably.  ;)  )

Comment 9

[Stephen P. Morse]

There are three symptoms grouped together in this report (see my comment of 
6-22) because I assumed they were all being caused by the same bug.  However, 
symptom #3 is not longer occuring (flickering between white and grayed out).  I 
suspect that perhaps Karnaze's changes for bug 43732 might have been responsible 
for fixing symptom #3.  However the first two symptoms still exist.

Comment 10

[Eric Pollmann]

So in addition to the "readonly doesn't work" bug, there is also this:

- it takes a relatively long time for display to appear (it used to be 
intantaneoulsy as soon as you selected the cookie

Does this mean the text inside the text field is not updated immediately?  I've 
seen similar problems on other pages with many text inputs: 
http://blueviper/forms/test.html is a good example.  Notice how the text fields 
are all updated at different times and quite a bit after the page loads.  This 
used to happen faster before.

Comment 11

[Stephen P. Morse]

Correct, the text inside the text fields are not updated instantaneously but 
rather take a noticible amount of time.  This may be related to bug 42471 -- 
that bug involves data that the user types into a text field whereas this one 
involves prefilled data in the text field.  But in both cases, there is a 
noticable delay as each character is entered.

Comment 12

[rubydoo123]

setting to m17

Comment 13

[kinmoz]

Assigning to [e-mail], taking from mjudge per beppe.

Comment 14

[kinmoz]

Accepting bug.

Comment 15

[kinmoz]

Adding ETA of 07/14.

Comment 16

[leger]

What things in the UI can be editted?  Is there a potential for an data loss in 
any of the UI fields?

Comment 17

[Stephen P. Morse]

The fields that can be modified are all the cookie properties -- it's name, 
value, host, expiration date, etc.  However the modifications mean nothing, you 
are not changing any of the cookie properties, you are only changing the display 
of the properties on the screen.  That is why they were given the read-only 
attribute in the xul.  But, unfortunately, that attribute is not being observed.

No, there's no potential for data loss, only for confusion if the user thinks he 
is really changing the cookie properties.

Comment 18

[John Morrison]

While there may be no potential for data loss in the Cookie UI, I should note
that *any* HTML or XUL text field that is intended to be readonly, is in fact
editable at this time. There is potential for data loss, particularly for an
HTML form that is relying on readonly state (admittedly that's uncommon).

Comment 19

[kinmoz]

Fix checked in:

    mozilla/layout/html/forms/src/nsGfxTextControlFrame2.cpp  revision 1.55

Modified CreateAnonymousContent() and AttributeChanged() to set the editor's
readonly and disabled flags.
r=[e-mail]

Comment 20

[kinmoz]

Attachment: My ReadOnly/Disabled TextWidget test case.

Comment 21

[Stephen P. Morse]

Pulled kin's changes and indeed it fixes the problem in the cookie viewer.  It 
is no longer possible to modify the contents of the cookie properties.  
Furthermore, I think the cookie properties are being updated quicker as well.  I 
don't recall just how "instantaneous" the update used to be before the 
regression started so I can't compare it, but it certainly seems acceptable now.

Thanks kin.

Comment 22

[Chris Petersen]

Fixed in the July 14th build.