Ticket #8288 (closed bug: fixed)

Opened 2 years ago

Last modified 2 years ago

Regression: jquery-ui animations focus fix causes Firefox Security Manager veto.

Reported by: coling Owned by: coling
Priority: minor Milestone: 1.8.21
Component: ui.effects.core Version: 1.8.20
Keywords: Cc:
Blocking: Blocked by:

Description

Hi,

I have found a regression with commit:  https://github.com/jquery/jquery-ui/commit/82df6924cbb0fa080590d83b4edc6183dd05ce93 (referencing #7595)

I can reproduce the error easily in my code, but it seems someone else also ran into the same issue:  https://github.com/mihaild/jquery-html5-upload/issues/2

Firebug Security manager text:

Security Manager vetoed action arg 0 [nsIDOMHTMLDivElement.contains]

Simply enclosing the whole check in a try/catch block works around the problem and is (to me) as sensible precaution anyway.

I'm happy to test any alternative fixes however.

I will upload a patch shortly.

Change History

comment:1 Changed 2 years ago by rdworth

  • Component changed from ui.core to effects.core

comment:2 Changed 2 years ago by scott.gonzalez

  • Owner set to coling
  • Status changed from new to pending

Please provide a reduced test case, pointing at an issue in an unrelated project isn't too helpful.

comment:3 Changed 2 years ago by coling

  • Status changed from pending to new

Sorry, I don't have time right now to produce a reduced test case (the conditions for it are no doubt complex and the code I have is pretty complex with various screen transitions), but I'm pretty certain the issue on the other project is caused by this same previous fix. When I find time I'll try and reduce it to a simple test case (current code involves iframe based form submission for file uploads and transitions, so I'd guess any test case would require server code also to deal with ajax responses).

My fix is here if you want it (it's pretty harmless with regards to the previous behaviour before #7595, so you may still want to apply it.  https://github.com/jquery/jquery-ui/pull/645

As I said, above, there may be a nicer fix but it works for me.

comment:4 Changed 2 years ago by scott.gonzalez

  • Status changed from new to pending

We need a test case or we can't consider this a bug.

comment:5 Changed 2 years ago by coling

  • Status changed from pending to new

OK, I managed to simplify the case where this shows up.

 http://jsfiddle.net/2efq2/

What should happen (and does under Chrome) is that when you select a file from the form, the box should slide way to the left, be replaced with a message that slides in from the right, and the form submitted.

What actually happens is that Firefox will veto the action with the above message. Tested on Firefox 10 ESR on linux and Firefox 11 Windows. Having Firebug opened when you load the above fiddle should show the security manager veto message.

comment:6 Changed 2 years ago by scott.gonzalez

  • Status changed from new to open

This only occurs if you click in the text portion of the input element, not the button. For some reason Firefox is removing the input element (which had focus) from the document.

comment:7 Changed 2 years ago by scott.gonzalez

An actually reduced test case:  http://jsfiddle.net/2efq2/8/

comment:8 Changed 2 years ago by coling

FWIW, it happens for me whether I click on the button or the text area of the upload element. This is on the Linux version which may have a slightly different layout for such elements however.

comment:9 Changed 2 years ago by gnarf

Confirmed as well -  http://jsfiddle.net/wfkxu/2/ gives a pure JS reduction of the use case, maybe this is a Firefox bug?

comment:10 Changed 2 years ago by coling

Could very well be a Firefox bug - certainly that test case is pretty damning. Can't think why it should be vetoed.

Assuming it is, what is your policy on that? I'm normally a kind of hard line on these things (only fix the issue where it should be fixed, don't add kludges), but as I've had people comment to me about the problem uploading content on Firefox after jQuery UI 1.8.16, I'd also quite like the try/catch workaround added upstream :p

comment:11 Changed 2 years ago by coling

Would you like me to open a bug report over at Mozilla?

comment:12 Changed 2 years ago by scott.gonzalez

@coling Thanks, I'm already working on it.

comment:13 Changed 2 years ago by scott.gonzalez

There's an existing Mozilla ticket, from two years ago with zero activity :-(  https://bugzilla.mozilla.org/show_bug.cgi?id=561664

I've added a comment with links to this ticket, and multiple reduced test cases. I'll also ping individual Mozilla devs about it.

comment:14 Changed 2 years ago by coling

Good sleuthing on finding the existing bug. I've added myself to the CC there.

As I don't really expect to be holding my breath for a fix and as this was exposed by a previous bugfix in jQuery, do you think adding the try/catch block as I suggested would be sensible in the short term?

As I mentioned above, I don't normally like these kind of kludges if the problem is genuinely elsewhere, but I guess your policy on this is usually somewhat more pragmatic (otherwise support for older browsers would be dropped a long time ago!!)

Obviously it's in our interests to close this bug (either as wontfix/upstream or as as workaroundcommitted - I don't know your resolutions, so I'm just making them up!) as it will probably not serve much purpose on it's own and will just detract from your other reports.

comment:15 Changed 2 years ago by scott.gonzalez

We'll definitely land a fix for this, probably this week. We're just trying to figure out the cleanest way to detect and work around this bug.

comment:16 Changed 2 years ago by coling

Awesome, I'll just shut up and let you get on with it then :)

Please do ping me if you need any input on it, but yu all likely understand the issue far better than me anyway.

Cheers and KUTGW!

comment:17 Changed 2 years ago by Scott González

  • Status changed from open to closed
  • Resolution set to fixed

Effects: Check for anonymous content being exposed via document.activeElement. Fixes #8288 - Regression: jquery-ui animations focus fix causes Firefox Security Manager veto.

Changeset: a7e143b4fe395baeda297ea96ef39921ea7f5a5d

comment:18 Changed 2 years ago by Scott González

Effects: Check for anonymous content being exposed via document.activeElement. Fixes #8288 - Regression: jquery-ui animations focus fix causes Firefox Security Manager veto. (cherry picked from commit a7e143b4fe395baeda297ea96ef39921ea7f5a5d)

Conflicts:

ui/jquery.effects.core.js

Changeset: b676d5956137d8bc35087e3160813a025be436d0

comment:19 Changed 2 years ago by scott.gonzalez

  • Milestone changed from 1.9 to 1.8.21
Note: See TracTickets for help on using tickets.