Closed Bug 355362 Opened 18 years ago Closed 18 years ago

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

Categories

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

x86
Windows XP
defect

Tracking

()

VERIFIED FIXED
mozilla1.9alpha1

People

(Reporter: martijn.martijn, Assigned: bzbarsky)

References

()

Details

(Keywords: regression, testcase)

Attachments

(2 files)

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.
Attached file testcase (deleted) —
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....
The alert isn't being stopped. Instead, the onchange event handler isn't fired.
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?
(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.
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...
Flags: blocking1.9?
Yeah, to see the bug, you have to click on the textfield, I wasn't aware of that yet, when I wrote the testcase.
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?
*** Bug 355931 has been marked as a duplicate of this bug. ***
From the duplicate bug apparently this crashes now. Please see the duplicate for the talkbackID's and stack traces.
Keywords: crash
The duplicate bug is not a duplicate. The only thing in common is they happen on the same page.
Keywords: crash
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 {
What's the stack to that SetValue() call?
Attached patch Patch to test (deleted) — Splinter Review
Martijn, does this help?
(In reply to comment #14) > Martijn, does this help? Yes, that makes the bug go away.
Flags: blocking1.8.1.1?
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.
Attachment #242190 - Flags: superreview?(jst)
Attachment #242190 - Flags: review?(jst)
Assignee: nobody → bzbarsky
Priority: -- → P2
Summary: Google pages file uploader doesn't work anymore → [FIX] Google pages file uploader doesn't work anymore
Target Milestone: --- → mozilla1.9alpha
Comment on attachment 242190 [details] [diff] [review] Patch to test r+sr=jst
Attachment #242190 - Flags: superreview?(jst)
Attachment #242190 - Flags: superreview+
Attachment #242190 - Flags: review?(jst)
Attachment #242190 - Flags: review+
Fixed on trunk.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Comment on attachment 242190 [details] [diff] [review] Patch to test Should probably fix this regression on branch too....
Attachment #242190 - Flags: approval1.8.1.1?
(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.
Blocks: 258875
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?
btw, I'd test comment 21 myself, except I can't get any accesskeys to trigger on branch at all.
(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?
Ah, indeed. Looks like triggering the accesskey just focuses the textfield on branch. So we're good.
Flags: blocking1.9?
Flags: blocking1.8.1.1?
Attachment #242190 - Flags: approval1.8.1.1?
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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: