875252 - Google+ "Upload from computer" unclickable on Firefox

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect, P2)

Description

[Benjamin Kerensa [:bkerensa]]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:22.0) Gecko/20100101 Firefox/22.0 (Beta/Release)
Build ID: 20130522153407

Steps to reproduce:

Try to click on Upload from computer on Google+


Actual results:

The button/element is unclickable


Expected results:

It should be clickable and launch a file browser so I can select and upload photos to Google+

Comment 1

[Benjamin Kerensa [:bkerensa]]

Here is a video showing the issue.... I re-installed Firefox on Ubuntu and checked in two other browsers and can confirm this is specific to Firefox 22 on Ubuntu but also did not occur in earlier versions of Firefox.

http://www.youtube.com/watch?v=1IyiOGXjxW4

Comment 2

[Loic]

Could you run mozregression to find a possible regression range and provide the changeset, please.
http://harthur.github.io/mozregression/

Comment 3

[Benjamin Kerensa [:bkerensa]]

I was unable to find a regression and more specifically I think this is related to the recent major overhaul Google+ did. I think they changed something in their design thats just not meshing well with FF.

Comment 4

[Steve Fink [:sfink] [:s:]]

Attachment: Allows spaces within filter expressions

Comment 5

[Steve Fink [:sfink] [:s:]]

Sorry! Wrong bug.

Comment 6

[Ekanan Ketunuti]

Good: 20130327031035
http://hg.mozilla.org/mozilla-central/rev/178a4a770bb1
http://download.cdn.mozilla.net/pub/mozilla.org/firefox/nightly/2013/03/2013-03-27-03-10-35-mozilla-central/

Bad: 20130328031013
http://hg.mozilla.org/mozilla-central/rev/962f5293f87f
http://download.cdn.mozilla.net/pub/mozilla.org/firefox/nightly/2013/03/2013-03-28-03-10-13-mozilla-central/

Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=178a4a770bb1&tochange=962f5293f87f

Comment 7

[Alice0775 White]

Try Aurora23.0a2. this is duplicate of Bug 52500 if Aurora works fine.
(I could not verify the problem because I am no Google+ account)

Comment 8

[Ekanan Ketunuti]

(In reply to Alice0775 White from comment #7)
> Try Aurora23.0a2. this is duplicate of Bug 52500 if Aurora works fine.
> (I could not verify the problem because I am no Google+ account)

Nooo. I can reproduce /w Nightly as well.
Mozilla/5.0 (Windows NT 6.1; rv:24.0) Gecko/20130526 Firefox/24.0

Comment 9

[Supphachoke Suntiwichaya]

Confirm I can't upload from G+ and Facebook. Firefox 22beta Linux and OS X

Comment 10

[Ekanan Ketunuti]

Alice, can you please activate G+ account and narrow down the regression range?

Comment 11

[Alice0775 White]

last Good: f372d9cf7d5c
First Bad: 9e55e669d3ac

Triggered by:
9e55e669d3ac	Mounir Lamouri — Bug 838695 - Move nsFileControlFrame click handling to nsHTMLInputElement. r=smaug

Comment 12

[Mounir Lamouri (:mounir)]

Is that still reproducible in current Nightly? I landed something that, I believe, should have fixed that kind of problems.

Comment 13

[Alice0775 White]

(In reply to Mounir Lamouri (:mounir) from comment #12)
> Is that still reproducible in current Nightly? I landed something that, I
> believe, should have fixed that kind of problems.

I can reproduce the problem in latest Nightly
http://hg.mozilla.org/mozilla-central/rev/a39263b0c896
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20130527 Firefox/24.0 ID:20130527031027
and
http://hg.mozilla.org/mozilla-central/rev/a39263b0c896
Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20130527 Firefox/24.0 ID:20130527031027

Comment 14

[Ekanan Ketunuti]

(In reply to Mounir Lamouri (:mounir) from comment #12)
> Is that still reproducible in current Nightly? I landed something that, I
> believe, should have fixed that kind of problems.

Still reproducible on mozilla-inbound changeset ce60c6798292
Mozilla/5.0 (Windows NT 6.1; rv:24.0) Gecko/20130528 Firefox/24.0

Comment 15

[Mounir Lamouri (:mounir)]

Sorry, as Alice in comment 7, I assumed the problem was coming from bug 52500. I will need a reduced test case to look into this.

Comment 16

[Alice0775 White]

I cannot provide reduced test case.

(I think that Google person is best to prepare testcase :)

Comment 17

[Alex Keybl [:akeybl]]

(In reply to Mounir Lamouri (:mounir) from comment #15)
> Sorry, as Alice in comment 7, I assumed the problem was coming from bug
> 52500. I will need a reduced test case to look into this.

Please don't wait on a reporter to help with a test - it's really important that we reproduce locally and start investigating as soon as possible, given the size of this web property.

Comment 18

[Alice0775 White]

Attachment: testcase (not sure whether exactly same as behavior of G+)

Comment 19

[Alice0775 White]

Attachment: testcase (not sure whether exactly same as behavior of G+)

Comment 20

[Benjamin Kerensa [:bkerensa]]

(In reply to Alex Keybl [:akeybl] from comment #17)
> (In reply to Mounir Lamouri (:mounir) from comment #15)
> > Sorry, as Alice in comment 7, I assumed the problem was coming from bug
> > 52500. I will need a reduced test case to look into this.
> 
> Please don't wait on a reporter to help with a test - it's really important
> that we reproduce locally and start investigating as soon as possible, given
> the size of this web property.

Hi Alex,

Let me know how to produce a testcase. I am currently now seeing Facebook image upload also does not work although that could be a seperate bug. But yeah the severity of this is pretty high considering the amount of people who visit these two properties.

Comment 21

[Olli Pettay [:smaug][[e-mail]]]

Benjamin, could you check if the Facebook problem has the same regression range?

Comment 22

[Mounir Lamouri (:mounir)]

Attachment: Reduced testcase

Comment 23

[Mounir Lamouri (:mounir)]

The problem is that the created HTMLInputElement doesn't have any presContext so we stop early in ::PreHandleEvent() and ::PostHandleEVent(). The reasons why we have those limitations and how easy it would be to remove them are not really known to me. Olli probably knows more.

I assume that fixing this for Beta is probably not a good idea. I think there would be two alternatives for Beta (and Aurora?): backing out the patches or override ::Click() to show a file picker if the element doesn't have a prescontext at that time. Olli, would that be a safe workaround?

Comment 24

[Mounir Lamouri (:mounir)]

Related bugs are:
- bug 234455 which started preventing events when there is no prescontext;
- bug 329509 which seems to be about stopping that.

Comment 25

[Olli Pettay [:smaug][[e-mail]]]

So how did this work before the changes? bug 329509 is definitely not something for beta.

Comment 26

[Olli Pettay [:smaug][[e-mail]]]

Could we just put back the old nsHTMLInputElement::Click() ?

Comment 27

[Mounir Lamouri (:mounir)]

(In reply to Olli Pettay [:smaug] from comment #26)
> Could we just put back the old nsHTMLInputElement::Click() ?

As said in comment 23, I believe this could work. I would be fine with backing out the required patch for Beta and think a bit more for Aurora/Nightly.

Note: I am unassigning myslef given that I will be mostly offline until the 12th, feel free te reassign me if it is fine having this being looked at in 10 days.

Comment 28

[Olli Pettay [:smaug][[e-mail]]]

I think we have to fix this asap.

Comment 29

[Benjamin Kerensa [:bkerensa]]

(In reply to Olli Pettay [:smaug] from comment #21)
> Benjamin, could you check if the Facebook problem has the same regression
> range?

I think the FB issue was just a glitch because its working for me now and I have not had any version change on FF.

Comment 30

[Benjamin Kerensa [:bkerensa]]

(In reply to Olli Pettay [:smaug] from comment #28)
> I think we have to fix this asap.

I totally agree I had hoped there would be a patch out for this by today since its been a few weeks now that I have actually had to use Chromium for photo uploads on G+ and I imagine some users could be contemplating switching completely.

Notably a few linux distros like Ubuntu are shipping Firefox 22.0 as stable so this is affecting a very large amount of users probably in the millions.

Comment 31

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Mounir is on vacation, so jst could you review?
Patches for Bug 838695 and 
http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/nsGenericHTMLElement.cpp?rev=ff4d7b275942&mark=2807-2807,2813-2820#2806
may help reviewing.
I'll write mochitest tomorrow.

Comment 32

[Olli Pettay [:smaug][[e-mail]]]

https://tbpl.mozilla.org/?tree=Try&rev=f50b20c34359

Comment 33

[Olli Pettay [:smaug][[e-mail]]]

Attachment: with tests

https://tbpl.mozilla.org/?tree=Try&rev=be3374e9cab4

Comment 34

[Olli Pettay [:smaug][[e-mail]]]

Attachment: for aurora+beta

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 838695
User impact if declined: G+, for example, is a bit broken
Testing completed (on m-c, etc.): about to land
Risk to taking this patch (and alternatives if risky): Should be reasonable safe.
Effectively bringing back the old behavior 
String or IDL/UUID changes made by this patch: NA

Comment 35

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d09f920a86f6

Comment 36

[Supphachoke Suntiwichaya]

lastest from me.

FF 22beta
Debian Linux
Linux mrchoke-debian 2.6.32-5-amd64 #1 SMP Fri May 10 08:43:19 UTC 2013 x86_64 GNU/Linux

Google+ not work.
Facebook work fine.

Comment 37

[Olli Pettay [:smaug][[e-mail]]]

This landed to mozilla-inbound yesterday, and should land to mozilla-central today, so
the next nightly, or nightly after that should have the fix.
Then the patch needs approval before it can land to aurora and beta.

Comment 38

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d09f920a86f6

Comment 39

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/afa4b9a24f20
https://hg.mozilla.org/releases/mozilla-beta/rev/7a0168d559d8

Comment 40

[Simona Badau, Desktop QA]

Verified as fixed on Firefox 22 beta 4:
Mozilla/5.0 (Windows NT 6.1; rv:22.0) Gecko/20100101 Firefox/22.0
Mozilla/5.0 (X11; Linux i686; rv:22.0) Gecko/20100101 Firefox/22.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:22.0) Gecko/20100101 Firefox/22.0
Build ID: 20130605070403

Comment 41

[Benjamin Kerensa [:bkerensa]]

Verified as fixed on Firefox 22 beta 4 (22.0~b4+build1-0ubuntu1):
Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:22.0) Gecko/20100101 Firefox/22.0

Comment 42

[Simona Badau, Desktop QA]

Verified as fixed on Firefox 23 beta 8:
Mozilla/5.0 (Windows NT 6.1; rv:23.0) Gecko/20100101 Firefox/23.0
Mozilla/5.0 (X11; Linux i686; rv:23.0) Gecko/20100101 Firefox/23.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:23.0) Gecko/20100101 Firefox/23.0
Build ID: 20130722172257

Comment 43

[Bogdan Maris, Desktop Test Engineering]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:24.0) Gecko/20100101 Firefox/24.0
Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20100101 Firefox/24.0
Mozilla/5.0 (Windows NT 6.2; rv:24.0) Gecko/20100101 Firefox/24.0

Verified as fixed on Firefox 24 beta 6 (buildID: 20130826142034) and latest Nightly (buildID: 20130826190748).