355362 - [FIX] Google pages file uploader doesn't work anymore

Status: VERIFIED FIXED

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

Description

[Martijn Wargers (dead)]

The file uploader for Google pages doesn't work anymore.
I'll attach a testcase, which I think describes the area of what regressed.
This regressed on trunk between 2006-07-05 and 2006-07-06.
There is no problem on branch.
I think this is a regression from bug 313337 somehow, although the patch for that bug is also checked in on the 1.8.1 branch.

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase

Comment 2

[Boris Zbarsky [:bzbarsky]]

Er...  With a trunk linux build I get the alert.  Do you not get it on Windows?  Popup blocking shouldn't be stopping the alert....

Comment 3

[Martijn Wargers (dead)]

The alert isn't being stopped. Instead, the onchange event handler isn't fired.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Er...  It's not fired?  If you try to mutate the DOM in that handler nothing happens?

What's the regression range for this on trunk including hours?

Comment 5

[Martijn Wargers (dead)]

(In reply to comment #4)
> Er...  It's not fired?  If you try to mutate the DOM in that handler nothing
> happens?

Indeed, nothing happens.

> What's the regression range for this on trunk including hours?

Between 2006-07-05 04 and 2006-07-06 04
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-07-05+04&maxdate=2006-07-06+04&cvsroot=%2Fcvsroot
When clicking on the 'Browse...', I don't see this bug, so that would explain why I don't see this on the 1.8.1 branch, because there you only get to see the filepicker, when clicking on the 'Browse...' button, not on the textcontrol frame.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Oh, so to get onchange to not fire you have to click on the textfield? Or tab to the control?  Or something else?  Testcase doesn't say...

Comment 7

[Martijn Wargers (dead)]

Yeah, to see the bug, you have to click on the textfield, I wasn't aware of that yet, when I wrote the testcase.

Comment 8

[Boris Zbarsky [:bzbarsky]]

OK.  So on Linux I still get the alert...  Is the focus event order different on Windows?  What happens when you trace the text control frame code?

Comment 9

[Ryan Jones-Ward [:sciguyryan]]

*** Bug 355931 has been marked as a duplicate of this bug. ***

Comment 10

[Ryan Jones-Ward [:sciguyryan]]

From the duplicate bug apparently this crashes now. Please see the duplicate for the talkbackID's and stack traces.

Comment 11

[Boris Zbarsky [:bzbarsky]]

The duplicate bug is not a duplicate.  The only thing in common is they happen on the same page.

Comment 12

[Martijn Wargers (dead)]

In a debug build, I hit these breakpoints when clicking on the text input of the file control:
Breakpoint 2, nsTextControlFrame::SetValue (this=0x137848b4, aValue=@0x22e6e
    at c:/mozilla/mozilla/layout/forms/nsTextControlFrame.cpp:2774
2774            InitFocusedValue();
Current language:  auto; currently c++
(gdb) cont
Continuing.

Breakpoint 1, nsTextControlFrame::CheckFireOnChange (this=0x137848b4)
    at c:/mozilla/mozilla/layout/forms/nsTextControlFrame.cpp:2579
2579    {
(gdb) cont
Continuing.

Breakpoint 1, nsTextControlFrame::CheckFireOnChange (this=0x137848b4)
    at c:/mozilla/mozilla/layout/forms/nsTextControlFrame.cpp:2579
2579    {

When I click on the "Browse..." button, I only hit this breakpoint:
Breakpoint 1, nsTextControlFrame::CheckFireOnChange (this=0x137848b4)
    at c:/mozilla/mozilla/layout/forms/nsTextControlFrame.cpp:2579
2579    {

Comment 13

[Boris Zbarsky [:bzbarsky]]

What's the stack to that SetValue() call?

Comment 14

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch to test

Martijn, does this help?

Comment 15

[Martijn Wargers (dead)]

(In reply to comment #14)
> Martijn, does this help?

Yes, that makes the bug go away.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 242190 [details] [diff] [review]
Patch to test

Sounds like on Windows the textfield remains focused while the filepicker is up... on Linux it does not.

Comment 17

[Johnny Stenback (:jst)]

Comment on attachment 242190 [details] [diff] [review]
Patch to test

r+sr=jst

Comment 18

[Boris Zbarsky [:bzbarsky]]

Fixed on trunk.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 242190 [details] [diff] [review]
Patch to test

Should probably fix this regression on branch too....

Comment 20

[[e-mail]]

(In reply to comment #19)
>(From update of attachment 242190 [details] [diff] [review])
>Should probably fix this regression on branch too....
Except this a regression from bug 258875 which never landed on the branch.

Comment 21

[Boris Zbarsky [:bzbarsky]]

Oh, hmm.  Yeah, bug 25887 might in fact be needed to reproduce...

What happens on branch if the file input has an accesskey?  Does triggering it put up the filepicker, or just focus the textfield?

Comment 22

[Boris Zbarsky [:bzbarsky]]

btw, I'd test comment 21 myself, except I can't get any accesskeys to trigger on branch at all.

Comment 23

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #22)
> btw, I'd test comment 21 myself, except I can't get any accesskeys to trigger
> on branch at all.

Because of bug 349716, perhaps?

Comment 24

[Boris Zbarsky [:bzbarsky]]

Ah, indeed.  Looks like triggering the accesskey just focuses the textfield on branch.  So we're good.

Comment 25

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED for me using build 2006-11-13-09 of SeaMonkey trunk under Windows XP; I get an alert saying "changed" when I select a file to upload.