Closed Bug 15170 Opened 25 years ago Closed 25 years ago

[GFX]Clicking on different field names doesn't change entries

Categories

(Core :: Layout: Form Controls, defect, P3)

Product:
Core
Component:
Layout: Form Controls
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: paulmac, Assigned: pollmann)

Details

Attachments

(2 files)

Steve's test case
312 bytes, text/html
Details
99% solution
2.99 KB, patch
Details | Diff | Splinter Review 
In Wallet Contents, clicking on the different Field Names doesn't change the
Entries for Field field.

I just went to the amazon sample page, filled in a bunch of stuff, captured the
form then went to Wallet Contents. Only home.city entry, sunnyvale for :-) is
shown, no matter what Field Name I click on.
Status: NEW → ASSIGNED
Target Milestone: M10
DAMMMM!!!  This was working.  Somebody changed something in the infrastructure.
I'll generate a simply test case.

Marking this as M10 because this sounds like a serious regression, not just for
wallet but probably for other things as well.
OK, I have a simple test case that demonstrates this.  The following content
displays two select lists containing two values in each.  The first list has a
size of 2 so both values are always displayed.  The second has no size so it
drops down each time it is selected.  Clicking on values in the second list
cause the ONCHANGE hander to be executed which is the correct behavior (message
gets printed on the console).  However the ONCHANGE handler does not get
executed when clicking on values in the first list.

<HTML>
  <HEAD>
    <SCRIPT>
      function Changed() {
        dump("Got a change\n");
      }
    </SCRIPT>
  <BODY>
    <FORM>
      <TABLE>
        <TR>
          <TD>
            <SELECT size='2' ONCHANGE=top.Changed();>
              <option>first</option>
              <option>second</option>
            </SELECT>
            <SELECT ONCHANGE=top.Changed();>
              <option>first</option>
              <option>second</option>
            </SELECT>
          </TD>
        </TR>
      </TABLE>
    </FORM>
  <BODY>
</HTML>
Assignee: morse → harishd
Status: ASSIGNED → NEW
Assignee: harishd → pollmann
Eric, could you take a look at this.  The content model looks correct.
html refcount=3<
  head refcount=2<
    Text refcount=2<\n  >
    script refcount=3<
      Text refcount=2<\r\n    function Changed() {\r\n      dump("Got a change\n
");\r\n    }\r\n  >
    >
  >
  Text refcount=3<\n \n >
  body refcount=3<
    Text refcount=3<\n    >
    form refcount=5<
      Text refcount=3<\n     >
      select size=2 onchange=top.Changed(); refcount=6<
        option refcount=4<
          Text refcount=3<first>
        >
        option refcount=4<
          Text refcount=3<second>
        >
      >
      Text refcount=3<\n     >
      select onchange=top.Changed(); refcount=7<
        option refcount=4<
          Text refcount=3<first>
        >
        option refcount=4<
          Text refcount=3<second>
        >
      >
      Text refcount=3<\n \n\n>
    >
  >
>
Onchange seems to have been broken for selects.  Have either of you made changes
there lately?
Status: NEW → ASSIGNED
Severity: normal → major
Component: Autofill → HTML Form Controls
Summary: Clicking on different field names doesn't change entries → [GFX]Clicking on different field names doesn't change entries
This is a GFX specific bug.  The ListControlFrame isn't sending onchange
notifications, but Native widgets are working for me in viewer.  I'm attaching
Steve's testcase.
Attached file Steve's test case — 

    
    

    
  
The handler gets called through this path
nsListControlFrame::MouseUp
nsComboboxControlFrame::ListWasSelected
nsComboboxControlFrame::UpdateSelection
nsComboboxControlFrame::SelectionChanged

The problem is we are no longer getting mouse up events in the drop down list
box of a combo box because it is hidden on a mouse down.  Was this behaviour
changed recently?

    
    

    
  
what is the latest on this?  fixable in the next day or two?

    
    

    
  
Probably.  I'm looking at it now.

    
    

    
  
I have a fix for this working in my local tree.  It needs some polishing,
testing, and a code review before I'm ready for a checkin.

Note that the onchange is still not being sent for comboboxes on Linux because
they are being rolled up on mouse down's instead of mouse up's.  I'll see if I
can kill both of these birds with one stone.

    
    

    
  
Is this checked in yet?  If not, will take only if there is one more re-spin for
M10.  We'll keep ya posted, and on M10 til we know.

    
    

    
  
No, this is not in yet.  The solution I came up with fails in one border case
(multiple selects, selecting with shift held down, might receive an extra
onChange event).  If you want this solution anyway, I'm attaching the diff.
Otherwise, I'm rewriting this to function just like the native onChange
detection worked and will be done this evening.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        99% solution
        — 
          — Splinter Review
      

    


  

    
    

    
  
The "Combo boxes not working on Linux" bug was a little over my head.  I gave it
to Pavlov.  See it here at bug 15537.  I've not got onchange working everywhere
<SELECT> works.  Still need to do the testing/reviewing thing before checking
in.

    
    

    
  
s/not/now/  Geez.  They do work.  :)

    
  
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED

    
    

    
  
I just checked in a fix.  To verify, pull up Steve's attached test case.  Click
on 'second' in the left list box.  You should see "Change!" written to the
console.

This is the change:

http://cvs-mirror.mozilla.org/webtools/bonsai/cvsquery.cgi?module=SeaMonkeyAll&branch=HEAD&cvsroot=/cvsroot&date=explicit&mindate=939166854&maxdate=939166995&who=pollmann%25netscape.com

    
  
Status: RESOLVED → REOPENED

    
    

    
  
Well I verified that the test case works in the M10 candidate builds.
Unfortunately, the bug that it was originally filed for, dealing with Wallet
Contents, can't be tested because I'm now crashing when I go to
Edit/Wallet/Wallet Contents. Might be related? I'm re-opening the bug for
investigation. Note, the last M10 builds, from Sunday night, do not have the
crash.
Perhaps this change should be backed out of the M10 branch.

Here is the stack trace:


 Call Stack:    (Signature = nsListControlFrame::DisplaySelected ebefe8d7)

   nsListControlFrame::DisplaySelected

[d:\builds\seamonkey\mozilla\layout\html\forms\src\nsListControlFrame.cpp, line
545]
nsListControlFrame::SetContentSelected

[d:\builds\seamonkey\mozilla\layout\html\forms\src\nsListControlFrame.cpp, line
1220]

   nsListControlFrame::Reset

[d:\builds\seamonkey\mozilla\layout\html\forms\src\nsListControlFrame.cpp, line
1339]

   nsListControlFrame::Init

[d:\builds\seamonkey\mozilla\layout\html\forms\src\nsListControlFrame.cpp, line
981]

   nsCSSFrameConstructor::InitializeScrollFrame

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 4038]

   nsCSSFrameConstructor::ConstructSelectFrame

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 2950]

   nsCSSFrameConstructor::ConstructFrameByTag

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 3060]

   nsCSSFrameConstructor::ConstructFrame

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 4869]

   nsCSSFrameConstructor::ProcessChildren

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 7987]

   nsCSSFrameConstructor::ConstructTableCellFrameOnly

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 1771]

   nsCSSFrameConstructor::ConstructTableCellFrame

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 1682]

   nsCSSFrameConstructor::ConstructFrameByDisplayType

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 4411]

   nsCSSFrameConstructor::ConstructFrame

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 4899]

   nsCSSFrameConstructor::TableProcessChild

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 1853]

   nsCSSFrameConstructor::TableProcessChildren

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 1827]

   nsCSSFrameConstructor::ConstructTableRowFrameOnly

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 1572]

   nsCSSFrameConstructor::ConstructTableRowFrame

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 1521]

   nsCSSFrameConstructor::ConstructFrameByDisplayType

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 4404]

   nsCSSFrameConstructor::ConstructFrame

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 4899]

   nsCSSFrameConstructor::TableProcessChild

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 1853]

   nsCSSFrameConstructor::TableProcessChildren

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 1827]

   nsCSSFrameConstructor::ConstructTableGroupFrameOnly

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 1472]

   nsCSSFrameConstructor::ConstructTableGroupFrame

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 1367]

   nsCSSFrameConstructor::ConstructTableFrame

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 1106]

   nsCSSFrameConstructor::ConstructFrameByDisplayType

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 4370]

   nsCSSFrameConstructor::ConstructFrame

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 4899]

   nsCSSFrameConstructor::ProcessChildren

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 7987]

   nsCSSFrameConstructor::ConstructFrameByTag

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 3153]

   nsCSSFrameConstructor::ConstructFrame

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 4869]

   nsCSSFrameConstructor::ContentAppended

[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 5417]

   StyleSetImpl::ContentAppended

[d:\builds\seamonkey\mozilla\layout\base\src\nsStyleSet.cpp, line 864]

   PresShell::ContentAppended

[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 1724]

   nsDocument::ContentAppended

[d:\builds\seamonkey\mozilla\layout\base\src\nsDocument.cpp, line 1575]

   nsHTMLDocument::ContentAppended

[d:\builds\seamonkey\mozilla\layout\html\document\src\nsHTMLDocument.cpp, line
1044]

   HTMLContentSink::NotifyBody

[d:\builds\seamonkey\mozilla\layout\html\document\src\nsHTMLContentSink.cpp,
line 276]

   HTMLContentSink::WillInterrupt

[d:\builds\seamonkey\mozilla\layout\html\document\src\nsHTMLContentSink.cpp,
line 1770]

   CNavDTD::WillInterruptParse

[d:\builds\seamonkey\mozilla\htmlparser\src\CNavDTD.cpp, line 3116]

   nsParser::ResumeParse

[d:\builds\seamonkey\mozilla\htmlparser\src\nsParser.cpp, line 958]

   nsParser::Parse

[d:\builds\seamonkey\mozilla\htmlparser\src\nsParser.cpp, line 805]

   nsHTMLDocument::ScriptWriteCommon

[d:\builds\seamonkey\mozilla\layout\html\document\src\nsHTMLDocument.cpp, line
1834]

   nsHTMLDocument::Write

[d:\builds\seamonkey\mozilla\layout\html\document\src\nsHTMLDocument.cpp, line
1847]

   NSHTMLDocumentWrite

[d:\builds\seamonkey\mozilla\dom\src\html\nsJSHTMLDocument.cpp, line 1154]

   js_Invoke

[d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 672]

   js_Interpret

[d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 2250]

   js_Invoke

[d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 692]

   js_Interpret

[d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 2250]

   js_Invoke

[d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 692]

   js_Interpret

[d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 2250]

   js_Invoke

[d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 692]

   js_InternalCall

[d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 765]

   JS_CallFunction

[d:\builds\seamonkey\mozilla\js\src\jsapi.c, line 2652]

   nsJSContext::CallFunction

[d:\builds\seamonkey\mozilla\dom\src\base\nsJSEnvironment.cpp, line 234]

   nsJSEventListener::HandleEvent

[d:\builds\seamonkey\mozilla\dom\src\events\nsJSEventListener.cpp, line 107]

   nsEventListenerManager::HandleEvent

[d:\builds\seamonkey\mozilla\layout\events\src\nsEventListenerManager.cpp, line
1032]

   GlobalWindowImpl::HandleDOMEvent

[d:\builds\seamonkey\mozilla\dom\src\base\nsGlobalWindow.cpp, line 2791]

   nsWebShell::OnEndDocumentLoad

[d:\builds\seamonkey\mozilla\webshell\src\nsWebShell.cpp, line 3403]

   nsDocLoaderImpl::FireOnEndDocumentLoad

[d:\builds\seamonkey\mozilla\webshell\src\nsDocLoader.cpp, line 889]

   nsDocLoaderImpl::OnStopRequest

[d:\builds\seamonkey\mozilla\webshell\src\nsDocLoader.cpp, line 768]

   nsLoadGroup::SubGroupIsEmpty

[d:\builds\seamonkey\mozilla\netwerk\base\src\nsLoadGroup.cpp, line 122]

   nsLoadGroup::RemoveChannel

[d:\builds\seamonkey\mozilla\netwerk\base\src\nsLoadGroup.cpp, line 607]

   nsInputStreamChannel::OnStopRequest

[d:\builds\seamonkey\mozilla\netwerk\base\src\nsInputStreamChannel.cpp, line
331]

   nsOnStopRequestEvent::HandleEvent

[d:\builds\seamonkey\mozilla\netwerk\base\src\nsAsyncStreamListener.cpp, line
283]

   nsStreamListenerEvent::HandlePLEvent

[d:\builds\seamonkey\mozilla\netwerk\base\src\nsAsyncStreamListener.cpp, line
153]

   PL_HandleEvent
                                                      [plevent.c, line 542]

    
  
Resolution: FIXED → ---

    
    

    
  
Clearing Fixed resolution due to reopen.

    
  
Target Milestone: M10 → M11

    
    

    
  
Moving to M11.  paulmac, please Release Note for M10 at:
http://bugzilla.mozilla.org/show_bug.cgi?id=14872

    
  
Status: REOPENED → ASSIGNED

    
    

    
  
I'm removing this code from M11, and replacing it with improved logic, see bug
15841.  I'll close this bug out when 15841 is fixed.

    
  
Status: ASSIGNED → RESOLVED
Closed: 25 years ago → 25 years ago
Resolution: --- → FIXED

    
    

    
  
Closing this out, shouldn't crash here anymore.

    
  
Status: RESOLVED → VERIFIED

    
    

    
  
verified

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: