Closed Bug 1241171 Opened 4 years ago Closed 4 years ago

FormData should not force 'blob' as filename.

Categories

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

46 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox45 --- unaffected
firefox46 + fixed
firefox47 + fixed

People

(Reporter: baku, Assigned: baku)

References

Details

(Keywords: regression)

Attachments

(1 file)

Attached patch formData3.patchSplinter Review
Attachment #8710197 - Flags: review?(bugs)
Comment on attachment 8710197 [details] [diff] [review]
formData3.patch

Hopefully this is web compatible. Better to land early in the next cycle.
Attachment #8710197 - Flags: review?(bugs) → review+
The change made in Bug 1237595 breaks existing sites with an optional file field (https://boards.4chan.org/qa/thread/418502). So I hope this lands in 46 or is uplifted.
Can confirm that this is needed in 46 to unbreak sites like 4chan, but 45 seems unaffected.
Version: 39 Branch → 46 Branch
[Tracking Requested - why for this release]: See comment 3.

Note that this means that either we ignore what comment 2 said or we back out bug 1237595 on 46 or something....
Oh, and for future reference, it's generally a good idea to request tracking on regressions.  Or at least set the regression keyword...
Keywords: regression
I'm ok with landing this patch if this fixes the regression.
Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/ad6293d247cb
Backout jobs: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=ad6293d247cb

Jobs of failing push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=3a4b31e5c049
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=20477679&repo=mozilla-inbound
03:42:57     INFO - TEST-START | /XMLHttpRequest/formdata-blob.htm
03:42:57     INFO - PROCESS | 1862 | ++DOCSHELL 0x7f8e09d2d000 == 5 [pid = 1910] [id = 39]
03:42:57     INFO - PROCESS | 1862 | ++DOMWINDOW == 17 (0x7f8e08ef2800) [pid = 1910] [serial = 114] [outer = (nil)]
03:42:57     INFO - PROCESS | 1862 | ++DOMWINDOW == 18 (0x7f8e09f05000) [pid = 1910] [serial = 115] [outer = 0x7f8e08ef2800]
03:42:58     INFO - PROCESS | 1862 | ++DOMWINDOW == 19 (0x7f8e09f0e000) [pid = 1910] [serial = 116] [outer = 0x7f8e08ef2800]
03:42:58     INFO - TEST-UNEXPECTED-FAIL | /XMLHttpRequest/formdata-blob.htm | formdata with blob - assert_equals: expected "\nkey=blob:text/x-value:5," but got "key=value,\n"
03:42:58     INFO - do_test/</client.onreadystatechange<@http://web-platform.test:8000/XMLHttpRequest/formdata-blob.htm:20:9
03:42:58     INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1382:20
03:42:58     INFO - Test.prototype.step_func/<@http://web-platform.test:8000/resources/testharness.js:1406:20
03:42:58     INFO - EventHandlerNonNull*do_test/<@http://web-platform.test:8000/XMLHttpRequest/formdata-blob.htm:18:35
03:42:58     INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1382:20
03:42:58     INFO - do_test@http://web-platform.test:8000/XMLHttpRequest/formdata-blob.htm:16:5
03:42:58     INFO - @http://web-platform.test:8000/XMLHttpRequest/formdata-blob.htm:36:3
03:42:58     INFO - TEST-PASS | /XMLHttpRequest/formdata-blob.htm | formdata with named blob 
03:42:58     INFO - TEST-PASS | /XMLHttpRequest/formdata-blob.htm | formdata.append() should throw if value is string and file name is given 
03:42:58     INFO - TEST-OK | /XMLHttpRequest/formdata-blob.htm | took 1529ms
Flags: needinfo?(amarchesini)
https://hg.mozilla.org/mozilla-central/rev/9fa27883caa1
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Flags: needinfo?(amarchesini)
Tracking since this is a regression. Can you request uplift please since it looks like aurora 46 is affected?
Flags: needinfo?(amarchesini)
Comment on attachment 8710197 [details] [diff] [review]
formData3.patch

Approval Request Comment
[Feature/regressing bug #]: FormData spec is changed twice recently and we need this patch in order to have the latest changes. Without some website can be broken.
[User impact if declined]: Broken websites.
[Describe test coverage new/current, TreeHerder]: mochitests
[Risks and why]: none
[String/UUID change made/needed]: none
Flags: needinfo?(amarchesini)
Attachment #8710197 - Flags: approval-mozilla-aurora?
Comment on attachment 8710197 [details] [diff] [review]
formData3.patch

Compatibility issue, includes fixes for tests.
Please uplift to aurora.
Attachment #8710197 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Could we get this landed rather soon to unbreak web sites on Aurora.
Depends on: 1247538
Depends on: 1246375
Blocks: 1237595
Depends on: 1248150
Several bad regressions here. baku can you take a look? Should this be backed out or do you think you can fix the regressions?
Flags: needinfo?(amarchesini)
This bug was fixing regressions caused by Bug 1237595, so you would need to back out both together.
We have a working patch in bug 1246375.
Flags: needinfo?(amarchesini)
Great, thanks baku, I took the uplift there.  How about bug 1247538 ?
Flags: needinfo?(amarchesini)
I would say that that bug is a duplicate. But I would like somebody else to help me testing it.
Flags: needinfo?(amarchesini)
On today's nightly, this bug is back - empty file upload fields now have filename="blob" again, breaking the same websites.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
[Tracking Requested - why for this release]:
(In reply to Thomas Daede from comment #22)
> On today's nightly, this bug is back - empty file upload fields now have
> filename="blob" again, breaking the same websites.

Can you tell me more? I need an example to work on it. Thanks.
Flags: needinfo?(tdaede)
new FormData(formElement) seems to cause the issue (if for form has <input type="file">). Someone please file a new bug where we deal with the regression. We really need to get back to the behavior beta has.
I filed bug 1250148.  Re-resolving this one, since it was in fact fixed and the tests are still passing, so the remaining problem (whatever it is) is slightly different than whatever was fixed in this bug....
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Clearing ni because info was provided in bug 1250148.
Flags: needinfo?(tdaede)
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.