Closed Bug 644959 Opened 13 years ago Closed 13 years ago

Disabled button after async reload

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla6
Tracking Flags:
Tracking Status
firefox5 --- wontfix
firefox6 --- fixed
firefox7 --- fixed

People

(Reporter: tza, Assigned: mounir)

References

Details

(Keywords: regression, testcase, Whiteboard: [bugday-2011-05-27])

Attachments

(3 files)

Async loading - button disabled
3.66 KB, text/html
Details
Screenshot Firefox 3.6 - Firefox 4.0
270.95 KB, image/png
Details
Patch v1
5.84 KB, patch
hsivonen
: review+
jst
: approval-mozilla-aurora+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.15) Gecko/20110303 Ubuntu/10.10 (maverick) Firefox/3.6.15
Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0) Gecko/20100101 Firefox/4.0

Bug #592665 has a similar behaviour, but I have a different szenario.

I disable buttons via javascript before async form submit.
After the submit, the dom elements are cleaned up and dont exist anymore (document.getElementById('btn1') == null).
When I load the same form again (async), the html response does not contain the attribute disabled="disabled" on the buttons.

But when the form gets displayed again, the buttons are still disabled (the attribute is not set in the html response). Seems like it gets cached. 

Reproducible: Always

Steps to Reproduce:
1. Disable buttons via javascript before submit (async)
2. Affected dom elements are removed after submit
3. Reload form again (in the Html response, the buttons are enabled).
4. Buttons are disabled when displayed
Actual Results:  
Buttons still have attribute disabled="disabled"

Expected Results:  
The buttons should not be disabled.

Occurs on http only, but not on https.
If changing the form id before each request (eg. from test to test1) buttons wont be disabled.
Reporter, can you please provide a testcase? Thanks.
Did this occur with 3.6.x?
Keywords: testcase-wanted
(In reply to comment #2)
> Did this occur with 3.6.x?

No, only happens with Firefox 4, works in 3.6.x.


(In reply to comment #1)
> Reporter, can you please provide a testcase? Thanks.

I will, after the weekend.
Keywords: regression, regressionwindow-wanted
Version: unspecified → 4.0 Branch
In the html testcase, I switched from an async request to YUI localdatasource to fake the async load. 

Same example with real async request can be found under: http://demo.silbergrau.com/ffdemo/

Open html file or the url above and then:
1) Open the dialog
2) Close dialog with button
3) Open dialog again => Button will be disabled (FF4)

    
    

    
  
Confirmed on Build identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0) Gecko/20100101 Firefox/4.0
Status: UNCONFIRMED → NEW
Ever confirmed: true

    
    

    
  
The following tiny test case also illustrates the issue clearly:

<!DOCTYPE html>
<html>
 <head><title>FF4 refresh bug</title></head>
 <body>
  <form>
   <input type="button" value="Disable" id="x" onclick="document.getElementById('x').disabled = true;"/>
  </form>
  <p>Click <b>Disable</b> then F5 to refresh. The button is <i>still</i> disabled!</p>
 </body>
</html>

Bug present in FF 4.0.1 (20110413222027) on Win 7 64bit

    
    

    
  
If someone can invest some Time, you can help by finding a Regression Range using http://harthur.github.com/mozregression/
Keywords: testcase-wanted

    
    

    
  
I did a preliminary pass using mozregression, but the comment 5 and comment 7 testcases appear to have different regression ranges.

Reporter, please can you clarify exactly what is expected of the testcase in comment 5; Rotan please file comment 7 as a new bug and paste the link here - unless you can conform the regression ranges are the same (using the tool in comment 8).

Thanks :-)

    
  
Keywords: testcase

    
    

    
  


  
I would expect, that the button is Always enabled after async loading (looking at http://demo.silbergrau.com/ffdemo/). The screenshot shows difference between FF3.6 <-> FF4.0 after the first loading.

Looking at the response, there is no disabled flag for the button, but it is still disabled in FF4:
<button id="testButton" name="button" onclick="return disableAndRemove(this)">Disable and remove button</button>

    
    

    
  
Regression window of attachment 522309 [details] in comment 5:

Last good nightly: 2010-05-03 First bad nightly: 2010-05-04

Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=83c887dff0da&tochange=d6bb0f9e9519

I also found that setting html5.parser.enable to false changes the current Nightly from a reproduced one to a WFM:
Mozilla/5.0 (X11; Linux x86_64; rv:7.0a1) Gecko/20110527 Firefox/7.0a1

Thus it's probably bug 373864 in that regression range that makes the difference.
Whiteboard: [bugday-2011-05-27]

    
    

    
  
From the description, it seems that the form state is being loaded from the session history even when it shouldn't. What should the parser do here to tell the form state restoration mechanism that it's not wanted in this case?

    
    

    
  
(In reply to comment #7)
> The following tiny test case also illustrates the issue clearly:
> 
> <!DOCTYPE html>
> <html>
>  <head><title>FF4 refresh bug</title></head>
>  <body>
>   <form>
>    <input type="button" value="Disable" id="x"
> onclick="document.getElementById('x').disabled = true;"/>
>   </form>
>   <p>Click <b>Disable</b> then F5 to refresh. The button is <i>still</i>
> disabled!</p>
>  </body>
> </html>
> 
> Bug present in FF 4.0.1 (20110413222027) on Win 7 64bit

This issue is different and very likely has tons of bugs filed in bugzilla.

    
  
Component: General → HTML: Parser
Product: Firefox → Core
QA Contact: general → parser
Version: 4.0 Branch → Trunk

    
    

    
  
This is actually a bug in nsHTMLButtonElement.cpp. Taking this bug given that I have a patch fixing it locally. I will attach it as soon as I have a test.
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Component: HTML: Parser → DOM: Core & HTML
QA Contact: parser → general

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch v1
        — 
          — Splinter Review
      

    


  
Fixing the issue and creating a test suite checking form restoration when created from a parser fragment for all form elements that have restoration enabled.

        Attachment #536049 -
        Flags: review?(Olli.Pettay)

    
  
Whiteboard: [bugday-2011-05-27] → [bugday-2011-05-27][needs review]

    
    

    
  
I believe we shouldn't take this patch for FF5 but should we take it for FF6 given that it's a regression?
On the one hand, I would say it's an easy and simple fix. On the other hand, it doesn't seem to appears that often.

          status-firefox5:
          --- → affected

          status-firefox6:
          --- → affected

          tracking-firefox6:
          --- → ?

    
    

    
  
Comment on attachment 536049 [details] [diff] [review]
Patch v1

I think Henri should look at this.
(I can review too, if needed.)

        Attachment #536049 -
        Flags: review?(Olli.Pettay) → review?(hsivonen)

    
    

    
  
Comment on attachment 536049 [details] [diff] [review]
Patch v1

r=me.

        Attachment #536049 -
        Flags: review?(hsivonen) → review+

    
  

          tracking-firefox6:
          ? → ---

    
  
Whiteboard: [bugday-2011-05-27][needs review] → [bugday-2011-05-27]

    
  
Flags: in-testsuite+
Whiteboard: [bugday-2011-05-27] → [bugday-2011-05-27][fixed in cedar]

    
    

    
  
Pushed:
http://hg.mozilla.org/mozilla-central/rev/633b6373e876

tza, thank you for your bug report!
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [bugday-2011-05-27][fixed in cedar] → [bugday-2011-05-27]
Target Milestone: --- → mozilla7

    
    

    
  
Comment on attachment 536049 [details] [diff] [review]
Patch v1

See comment 16 for request reasons.

        Attachment #536049 -
        Flags: approval-mozilla-aurora?

    
    

    
  
Comment on attachment 536049 [details] [diff] [review]
Patch v1

Approved for Aurora 6.

        Attachment #536049 -
        Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

    
    

    
  
Pushed to mozilla-aurora:
http://hg.mozilla.org/releases/mozilla-aurora/rev/488cd9e29d65

This is going to be fixed in Firefox 6 then.

Marking wontfix for Firefox 5 given that it's not critical enough to push the fix to mozilla-beta IMO.

          status-firefox5:
          affected → wontfix

          status-firefox6:
          affected → fixed

          status-firefox7:
          --- → fixed

    
  
Target Milestone: mozilla7 → mozilla6

    
    

    
  
Mozilla/5.0 (Windows NT 6.1; rv:7.0) Gecko/20100101 Firefox/7.0

I followed the steps in comment 4 and it seems the issue was fixed.
Thanks.
Status: RESOLVED → VERIFIED

    
    

    
  
FF8 still exhibits the erroneous behaviour I described in comment 7.

I don't work on the moz code, but I know that those who do have some spare cycles to spend on this worthy task appreciate the benefit of a repeatable test. So please check the tiny piece of markup in comment 7 in your FF and see for yourselves.

I can't tell if this behaviour is related to the original issue, should be reported as a separate bug, or is simply another manifestation of a long-known problem. If it is related to the original issue, then bug 644959 has been closed prematurely.

    
    

    
  
(In reply to Rotan from comment #24)
> FF8 still exhibits the erroneous behaviour I described in comment 7.

The issues I described in the comments 1 and 10 were fixed. So I guess Rotan's issue must be something else. Therefore you might consider reopening bug 644959.

    
    

    
  
I have this bug. Situation:
We have button without attr disabled. Click this button start some routine and JS set attr disabled='disabled' to this button with jQuery 'button' plagin, which in Bootstrap 2. And after all routine finished, 'button' plugin reset this button and remove attr disabled. If we reload page before button reseted and attr disabled don't removed in firefox 19.0.2 that button will have attr disabled='', but behaviour like disabled='disabled' and any action not help: reload page, clean cash, etc...only new tab or reload firefox help.

In chrome/safari that good, i think that ffox cached this state, but clean cache in settings this not help.

Its situation rare, but possible, that should fixed. Thanks, with regards, Vadim.

    
    

    
  
forget, Mac OS X 10.8.2

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: