70742 - unable to focus, or enter text in some pref panel textfields if another pref panel is viewed first

Status: VERIFIED FIXED

(SeaMonkey :: Preferences, defect, P2)

Description

[ratman]

using win32 installer mtrunk build 2001030205 (the respin?).

un) open edit -> preferences
deux) select any preference panel other than the main navigator pref panel
trois) select the main navigator pref panel
quatre) click and/or start typing in the textarea in order to type in a url

expected behavior) textarea receives focus and/or words start appearing

actual behavior) rien [nada]


since the main navigator pref panel is the default one upon entering the
preferences, if you don't switch to a different pref item, the home page
textarea works just fine.  the bug occurs only if you first visit another panel
and return.  perfectly repeatable.  this is a new-for-today bug, i didn't see
this with yesterday's win32 mtrunk installer build.

Comment 1

[Fabian Guisset]

maybe it has gone away again?

Comment 2

[sairuh (rarely reading bugmail)]

ben or blake, would this belong to you? reassign as needed, thx!

i see this on mac and linux, too. nominating for beta1...also tempted to
nominate for moz0.8.1 [i know, it's last moment, but this is a rather annoying
regression].

Comment 3

[sairuh (rarely reading bugmail)]

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

Comment 4

[sairuh (rarely reading bugmail)]

also unable to bring up the context menu for this textfield [from bug 70887].

Comment 5

[sairuh (rarely reading bugmail)]

affects other textfields in prefs, eg Cache:

1. open prefs dialog.
2. go to Cache panel, notice that you can bring into focus and type in the
textfields.
3. go to another panel, eg, Appearance.
4. return to Cache panel and attempt to edit textfields.

result: unable to focus or edit textfields.

i went over this w/jrgm, who also sees the behavior, but oddly enough we found
that it wasn't reproducible in the Proxies panel. hrm.

hyatt/saari, would this belong to you?

Comment 6

[ajbu]

Also seeing this, Windows 2000, Mozilla 2001031120 installer. Classic and 
Modern theme
Maybe helpful:
- When going to Composer / 'New Page Settings', choose another pref-panel, and 
og back to the 'New Page Settings' page, the 'Author' text field stopped 
working, but the 'Background image' text field is still working.
- On a non-functional edit field, the mouse does not change to an I-beam when 
over the text field, but does change to an I-beam when over the text-field 
border.
- The text field is also removed as a tab-stop, when tabbing through the fields 
with the keyboard it doesn't go to the non-functional textfield.

Comment 7

[Paul Chen]

Marking nsbeta1+, p2, mozilla0.9

Comment 8

[saari (gone)]

CC'ing bryner since some view manager changes that are related to focus may be
involved here

Comment 9

[sairuh (rarely reading bugmail)]

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

Comment 10

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

up a level. not mine.

Comment 11

[sairuh (rarely reading bugmail)]

passin' over to mcafee, to see if he can fix it --or know the right person who
could. thx!

Comment 12

[sairuh (rarely reading bugmail)]

->aaronl, could you take this?

also nominating for moz0.9.1 --since i doubt a fix for this will make it to 0.9.
unless i'm wrong (which i wouldn't mind here ;) and someone here *does* have a
fix that'd cover this...

Comment 13

[ratman]

using win32 installer mtrunk build 20010411??

no fix here, but an interesting observation:

after displaying a panel, moving on, and then returning, the cursor over the
textfield is a standard one, instead of an i-bar.  the exception, however, is
the left and right edges of the textfield box, where not only does an i-bar
cursor appear, but so does a context menu, although there really isn't anything
to copy/cut/paste from a null selection (though selecting all and copying will
overwrite previous clipboard contents).

i really doubt it, but could this be a borders issue?

Comment 14

[Peter Trudelle]

One problem per bug report, please! Open new bug reports for other problems.  
This report is about an inability to type in the URL text field following steps 
un...quatre. I can reproduce this in today's NS installed bits on Win98, but not 
in mozilla builds from today. Reassigning to component owner to ensure that 
installer is getting everything right. clearing target for re-triage.

Comment 15

[ratman]

- actually, this bug expanded to the failure to focus, to bring up a context
menu, and to enter text into *several* preference panel textfields - please see
sairuh's previous comment regarding the cache panel and discussion with jrgm. 
summary is reworded to reflect this.

- my apologies for essentially dittoing ajbanck's previous comment with my last one.

- there may be a relation here with previous issues over the history pref panel
textfield - i'll dig up bug numbers soon.

Comment 16

[sairuh (rarely reading bugmail)]

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

Comment 17

[HJ]

The textfields keep working if you remove the 'parent.initPanel()' stuff out of
onload="" It seems that something in onpageload in nsPrefWindows.js, is the
cause or triggers this bug because I don't have this problem after taking it out!

Comment 18

[sairuh (rarely reading bugmail)]

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

Comment 19

[Fabian Guisset]

Timeless, eddyk, might this be due to your checkin on march 1st to enable
prefIsLocked stuff? Commenting out the patch doesn't fix the bug but maybe you
guys have an idea about this.

Comment 20

[timeless]

it shouldn't. But i don't know enough to be sure (nor am i awake enough to 
right a clear/accurate/polite comment).  If you think it's my bug, 
reassign to me, otherwise I have other stuff to do. Considering Ben didn't want 
the bug I'm going to assume i don't want it either.

Comment 21

[eddyk]

Arg.  I removed my changes on onpageload, but bug is still there.

I change the WSM attribute initialiser for disabled properties, and the bug 
seems to disappear.

this.wsm.attributes = ["pref", "preftype", "prefstring", "prefattribute", 
"disabled"];

to
this.wsm.attributes = ["pref", "preftype", "prefstring", "prefattribute"];

arg.

Another thing I just noticed while playing around with this is that context 
menus come up only once.  The second right click in a textfield doesn't do 
anything.

arg

Comment 22

[HJ]

Is that 'disabled' used for something like .setAttribute("disabled","fale") on a
<textbox> ?

note: contextmenu bug is bug 78725

Comment 23

[HJ]

Ok found the problem is generic_Set: in nsWidgetStateManager.js

generic_Set:
  function ( aElement, aDataObject ) {
    if( aElement ) {
      for( var property in aDataObject ) {
        aElement.setAttribute( property, aDataObject[property] );
        // this is HJ's small hack, check for element textbox 
        if ( aElement.nodeName == "textbox" && property == "disabled" ) {
           // Yes sir, textbox it is
          if( aDataObject[property] == "false" || DataObject[property] == "" ) {
            // We don't need crap, so remove this attribute
            aElement.removeAttribute(property);
          }
        }
      }
    }
  },

Sorry no diff but this will do the trick.

Comment 24

[eddyk]

It seems to work, BTW the code you posted has a typo.

However, I have a question.  Should the textbox be fixed?  It was in a 
semi-disabled state when the disabled property was set to a non-null, non-true 
value.  Either it should be disabled or be enabled, IMO.

Should another bug be filled to fix textfields instead of working around it in 
WSM?

Thanks!

Comment 25

[eddyk]

How about this diff instead (if this approach is the right one to take)?  This
would also take care of bug 76937.  Basically don't set disabled if it's an
empty string or false for any xul element.


Index: mozilla/xpfe/global/resources/content/nsWidgetStateManager.js
===================================================================
RCS file: /cvsroot/mozilla/xpfe/global/resources/content/nsWidgetStateManager.js,v
retrieving revision 1.16
diff -u -r1.16 nsWidgetStateManager.js
--- nsWidgetStateManager.js	2001/05/09 08:32:51	1.16
+++ nsWidgetStateManager.js	2001/05/09 23:51:46
@@ -192,6 +192,11 @@
               for( var property in aDataObject )
                 {
                   aElement.setAttribute( property, aDataObject[property] );
+                  if ( property == "disabled") {
+                    if (aDataObject[property]=="false" ||
aDataObject[property]=="") {
+                      aElement.removeAttribute(property);
+                    }
+                  }
                 }
             }
         },

Comment 26

[HJ]

Hey, we at least now know what the problem is :)

I have filed a bug for textboxes, bug 72157, but that one is marked 'Future' so
that won't help us. Hyatt told me on IRC that the widgets are going to be
changed in the near future. 

We could close this bug, and bug 76937, with this workaround. Or we could also
fix bug 72157 by doing this in xulBindings.xml#383. 

Comment 27

[HJ]

oops, make that xulBindings.xml#247 for <textbox>

Comment 28

[HJ]

Who can update the summary because the context menu bug is reported in bug
78725. Let focus on the focus/enter text thing here. 

The reason for this is that some people on IRC told me that this bug is invalid
because there are to may different issues reported in this bug!

Comment 29

[Chris McAfee]

eddyk

Comment 30

[eddyk]

have asked blake for r= and ben for sr=

Comment 31

[Blake Ross]

if ( property == "disabled")

Remove that extra space, please.

Looks okay otherwise, I guess. r=blake

Comment 32

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

I'm a style nazi...

+                  if ( property == "disabled") {
+                    if (aDataObject[property]=="false" ||
aDataObject[property]=="") {

try:

aDataObject[property] == "false" || aDataObject[property] == ""

(with spaces) 
Do you mean only empty string? Because if you also mean null or undefined, !
aDataObject[property] is more generic. But if you explicitly mean empty string 
what you have is fine. 

Otherwise, sr=ben. 

Comment 33

[eddyk]

After explaining to someone else how the fix worked, I realised that the check
was being done unnecessarily inside a loop.  So I move the check/removeAttribute
from inside the for loop, to after the loop.

Performance shouldn't be hit as badly now.

Comment 34

[eddyk]

Attachment: same idea, but code is moved outside a tight loop

Comment 35

[eddyk]

Attachment: oops, wrong patch file. Same thing as previous without stupid formatting.

Comment 36

[eddyk]

Fix checked in, marking fixed.

Great job H-H!

Comment 37

[eddyk]

It's late, I meant H-J of course.

Comment 38

[eddyk]

Attachment: original logic, new place.

Comment 39

[eddyk]

Going back to original check as done by H-J.  Using the simplified form seems to
be too generic, causing the disabled property to be removed overzealously.  This
breaks preflocking.

So this final attempt is a combination of the original logic, but moved outside
of the loop.

Comment 40

[eddyk]

reopening so that I can get sr again.

Comment 41

[Marek Z. Jeziorek]

Setting target milestone to 0.9.2 (check it in anytime, even before, when the
tree is open for). Per PDT triage.

Comment 42

[eddyk]

Attachment: diff of generic test using different model (tested ok on linux,win,mac)

Comment 43

[eddyk]

marking fixed

Comment 44

[sairuh (rarely reading bugmail)]

cool! vrfy fixed using opt comm bits:

winnt - 2001.05.17.10
linux and mac - 2001.05.17.08