Closed Bug 953388 Opened 11 years ago Closed 10 years ago

Facebook.com - Firefox Android displays duplicate gallery entries, uploads image twice when commenting due to node cloning

Categories

(Web Compatibility :: Site Reports, defect, P2)

ARM
Android
defect

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: iamjayakumars, Assigned: Harald)

References

()

Details

(Whiteboard: [contactready][country-all])

Attachments

(2 files)

Attached image screenshot
Hi, 

When uploading a photo in facebook status
*Choose Photo from Gallery 
*In status only photo (editing mode)

After upload, the same photo uploaded twice.
check the attachment
This is not something the front-end team of Firefox can fix.
Component: General → Mobile
Product: Firefox for Android → Tech Evangelism
Version: Firefox 26 → unspecified
Blocks: 759986
Summary: Firefox Android uploads photo twice in facebook → Facebook.com - Firefox Android displays duplicate gallery entries
Attachment #8351679 - Attachment description: FIREFOXX.png → screenshot
Attachment #8351679 - Attachment filename: FIREFOXX.png → firefox-screenshot.png
Putting Harald as it is a facebook bug. 
This might requires a bit more analysis too to know what is happening.
Flags: needinfo?(hkirschner)
Hardware: x86 → ARM
(In reply to iamjayakumars from comment #0)

> When uploading a photo in facebook status
> *Choose Photo from Gallery 
> *In status only photo (editing mode)
> 
> After upload, the same photo uploaded twice.

I've just created a test account and am not able to reproduce on my Geeksphone Peak running 1.2. Can you try again?
I can reproduce with Firefox beta for Android. Will try to investigate..
Firefox on Android says the Fiddler root is installed but for some reason still refuses to load Facebook through Fiddler (says security error). I'll have to figure out that first :-/
Assignee: nobody → hsteen
Status: NEW → ASSIGNED
Flags: needinfo?(hkirschner)
The image is indeed uploaded twice - once with an Ajax request (POST https://upload.facebook.com/_mupload_/photo/x/saveunpublished/?thumbnail_width=56&thumbnail_height=56&waterfall_id=fe1185f85b08d35d981d6cc4cf7c78ca&waterfall_app_name=web_m_touch&waterfall_source=composer_feed&__av=100006763626692&fb_dtsg=AQAr6WcC&m_sess=&__dyn=1Zz94E21xC2p93FQ8xO&__req=a&__ajax__=true&__user=100006763626692) and again with the FORM POST when the status update is submitted (POST https://upload.facebook.com/_mupload_/composer/?site_category=m_touch).

FB clones the input type=file to allow you to attach more than one image, and does not (apparently) expect the cloned element to retain the state of the clonee.
this was considered desirable behaviour back in 2004: bug 197294.. I guess it still is (though I CC Boris in case he wants to comment). The recommended fix is to simply set .value='' on the cloned input before adding it to the DOM. Code:

    i.prototype.cloneDOMNode = function() {
        "use strict";
        var j = this.$MPhotoUploadFileInput0;
        this.$MPhotoUploadFileInput0 = j.cloneNode(true);
        g.replace(j, this.$MPhotoUploadFileInput0);

as seen in https://fbstatic-a.akamaihd.net/rsrc.php/v2/yW/r/fyE3nmvODl4.js
> I guess it still is 

I believe so, yes.

The relevant spec here is http://dom.spec.whatwg.org/#concept-node-clone which says to:

  Run any cloning steps defined for node in other applicable specifications and pass copy,
  node, document and the clone children flag if set, as parameters. 

and then http://www.whatwg.org/specs/web-apps/current-work/multipage/the-input-element.html#the-input-element says:

  The cloning steps for input elements must propagate the value, dirty value flag,
  checkedness, and dirty checkedness flag from the node being cloned to the copy.

We get this right.  So does Presto Opera.  WebKit/Blink aren't following the spec here.  At least based on this test:

  <!DOCTYPE html>
  <input type="file">
  <input type="button" onclick="cloneMe()" value="Clone me">
  <script>
    function cloneMe() {
      var file = document.querySelector("input[type=file]");
      var file2 = file.cloneNode();
      file.parentNode.insertBefore(file2, file.nextSibling);
    }
  </script>

If I use a text input instead of a file input, all UAs I can get my hands on clone the value.  So this seems like just a WebKit bug with file inputs...
Assigning to Harald for following up with Facebook.

(Aside for Boris: I find we're slightly inconsistent in that if you clone an input type=file after a file was selected and set clone.value='', clone.files.length will still be 1. It causes no real problems, no file is uploaded, but it looks like a P5-until-some-site-actually-breaks bug is hanging around somewhere there)
Assignee: hsteen → hkirschner
Priority: -- → P2
> clone.files.length will still be 1.

Hallvord, I can't seem to reproduce this...  Mind attaching a testcase that shows that problem for you, or mailing it to me or something?
(In reply to Boris Zbarsky [:bz] from comment #12)
> > clone.files.length will still be 1.
> 
> Hallvord, I can't seem to reproduce this... 

Ops. Sorry, it was a test bug.. Your implementation is indeed a spectactle of correctness and beauty :=)
Whiteboard: [contactready][country-all]
(just adding to the summary because I never find this bug - sorry about the spam :-/)
Summary: Facebook.com - Firefox Android displays duplicate gallery entries → Facebook.com - Firefox Android displays duplicate gallery entries, uploads image twice when commenting due to node cloning
Harald, 

Could you help us find the right person at Facebook?
Thanks.
Flags: needinfo?(hkirschner)
Colby, this is a general bug in the mobile site. Could you move it over to your internal bug system? Thanks
Flags: needinfo?(hkirschner) → needinfo?(colby)
This issue is no more.
Its not uploading twice in FF31
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → WORKSFORME
Flags: needinfo?(colby)
Product: Tech Evangelism → Web Compatibility
Component: Mobile → Site Reports
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: