Open Bug 1174210 Opened 9 years ago Updated 2 years ago

Firefox not correctly re-hiding element on AJAX based form on re-submit

Categories

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

defect

Tracking

()

People

(Reporter: gb3, Unassigned, NeedInfo)

Details

(Keywords: testcase-wanted)

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:38.0) Gecko/20100101 Firefox/38.0
Build ID: 20150525141253

Steps to reproduce:

http://mmwtrademarks.com.au/freetrademarksearch/

Steps:

1. Don't fill out any fields, just upload a file that isn't allowed such as a text file, doc, exe etc... then hit submit. Then you should see an error UNDER the browse button.

2. Upload an allowed image such as a png

3. Hit submit again... now the error SHOULD be gone, but isn't.

The JS code that runs to do this is located at: http://mmwtrademarks.com.au/wp-content/themes/mmw2015/js/cf7.js


Actual results:

On re-submit when the error SHOULD have been hidden, it wasn't.


Expected results:

The error message under the browse button should have been hidden since there was no longer an error in the field.

This has been tested on Chrome and IE11, both of which show the correct behaviour by hiding the field on re-submit with an allowed filetype.

Allowed filetypes are: gif|png|jpg|jpeg|pdf
Product: Core → Firefox
Would be nice if this was at least acknowledged.
Thanks for reporting this issue. I can reproduce, but it's not clear that the problem is necessarily a bug in Firefox (and not in the web page) -- there are many differences between Firefox and other browsers, and not all of them are bugs in Firefox.

If you add a breakpoint at this line:
  $('#upload_logo .wpcf7-not-valid-tip-alt').hide();
..you'll see that it correctly hides the validation message.

Something must've gone wrong elsewhere. Unfortunately we don't have the resources to make reduced testcases for every site where Firefox appears to work incorrectly, so if you could reduce this to a simple self-contained testcase and attach it here in bugzilla, that would be very helpful. https://developer.mozilla.org/en-US/docs/Mozilla/QA/Reducing_testcases

Thanks again and sorry it took so long for someone to look at this!
Flags: needinfo?(gb3)
Keywords: testcase-wanted
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:46.0) Gecko/20100101 Firefox/46.0
Nightly 46.0a1 Build ID: 20160105030232

This issue is reproducible. As said above not sure if this is Firefox issue or the site itself.
Status: UNCONFIRMED → NEW
Component: Untriaged → File Handling
Ever confirmed: true
Mozregression is giving a regression window of http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9e31df64bfd7&tochange=e0f6db50231f for this. Maybe sicking's DOMNodeRemoved changes?

Not sure how realistic a reduced testcase is here.
Component: File Handling → DOM
Flags: needinfo?(jonas)
Product: Firefox → Core
Version: 38 Branch → unspecified
There are lots of mutation event changes in that range. And the js file mentioned in comment 0 uses "DOMSubtreeModified" events. So it's quite possible that that's the culprit.

Sadly, if it is, I wouldn't expect that we'd roll back those changes. They dramatically simplified our mutation events handling, and removed a class of bugs that we haven't had at all since then.

Also, given that this is the first regression I've heard of in the 4.5 years since those patches landed, it's quite likely that any changes (or backouts) would cause more problems than it'd fix.

So, given the almost 5 year old regression, unless we get a reduced testcase that shows otherwise, or we find more websites that exhibit the same problem, I think we should chalk this up as a website problem.

Sadly specifications in this area are very vague and poorly written (and old). What we are doing is well within what the specification says. Unfortunately I suspect that other browsers behave somewhat different, but are also within specification.

There's been some debate lately about more precisely specify mutation events more exactly. Google has expressed interest in changing their implementation to match ours, since what we are doing is much easier and safer to implement.
Flags: needinfo?(jonas)
Component: DOM → DOM: Core & HTML
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.