Closed Bug 684506 Opened 8 years ago Closed 8 years ago

Nightly is using old file manager window

Categories

(Core :: Widget: Win32, defect)

All
Windows 7
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla9
Tracking Status
firefox8 --- fixed

People

(Reporter: Terepin, Assigned: bbondy)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110903 Firefox/9.0a1
Build ID: 20110903030832

Steps to reproduce:

Tried to download/upload a file.


Actual results:

Old file manager window appeared.


Expected results:

Current file manage window should opened, just like before.
Keywords: regression
Hardware: x86_64 → All
Confirming on Windows 7 64-bit with the new 20110903 build.
Regression wiondow(cached m-c hourly),
Works:
http://hg.mozilla.org/mozilla-central/rev/7d3d1c2c75f8
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110901 Firefox/9.0a1 ID:20110901004809
Fails:
http://hg.mozilla.org/mozilla-central/rev/ce43a8644bc0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110902 Firefox/9.0a1 ID:20110902030838
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7d3d1c2c75f8&tochange=ce43a8644bc0
Status: UNCONFIRMED → NEW
Component: General → Widget: Win32
Ever confirmed: true
Product: Firefox → Core
QA Contact: general → win32
Windows 7 32-bit
Nightly 20110903
Changeset: 76462:472716252ea3

Confirmed. Old-style file manager windows is shown.
Assignee: nobody → netzen
Blocks: 295540
Duplicate of this bug: 684524
As soon as you specify a hook via OFN_ENABLEHOOK this happens to the look.

Apparently Windows will not allow you to use a hook procedure via OFN_ENABLEHOOK with the new look.  This is because a lot of legacy hook code actually reorganized the file dialog's content and the new look would have broken.  Why they didn't just add another flag I'm not sure. 

We can simply NOT do a hook in Vista and up since the problem in Bug 295540 is only on XP anyway.  This would solve the problem.   

I just found out that this problem has actually been in Nightly for a while now but only in multi file selections because of Bug Bug 660833. I guess we can't get that fix for Vista and up until we move the new common file dialogs code.
As mentioned in Comment 5:
- This patch will have no side effects for the recent Bug 295540.
- This patch will cause the fix in Bug 660833 to only work in XP and 2000, but if we want the new looking dialogs we have no choice.  It will be fixed again once we start using the Common File Dialogs for Vista/7. 

I think it's better to have the search bar and new nav bar than to have the fix in Bug 660833 for multi selections.
Attachment #558094 - Flags: review?(jmathies)
Status: NEW → ASSIGNED
Duplicate of this bug: 684556
how did this even happen? it was fine before. What change really motivated this?
As per Comment 5 there was a fix for Windows XP which required us to pass an extra flag.  Windows Vista and 7 don't handle this flag properly in the new dialogs so it was falling back to the old dialog silently.  It was not noticed during development because the bug that introduced the flag was tested and done for Windows XP which is the same as the file picker dialog that you see now.
Attached file Description (obsolete) —
Attachment #558304 - Flags: ui-review+
Attachment #558304 - Flags: superreview?
Attachment #558304 - Flags: review+
Attachment #558304 - Flags: feedback+
Attachment #558304 - Flags: checkin+
Attachment #558304 - Flags: approval1.9.2.21?
Attachment #558304 - Flags: approval1.9.2.20?
Attachment #558304 - Flags: approval-mozilla-release?
Attachment #558304 - Flags: approval-mozilla-beta?
Attachment #558304 - Flags: approval-mozilla-aurora?
Serious question: How did this get uploaded?
(In reply to Peter Henkel [:Terepin] from comment #11)
> Serious question: How did this get uploaded?

my points exactly Terepin!!!
Attachment #558304 - Attachment is patch: false
Hi Peter & remixedcat.  

The answer was given in Comment 9 and Comment 5. 

Nightly builds are a wild place, they may or may not work.  
Nightly builds are the place where testing happens, so regressions are common and expected.

Firefox Aurora or Firefox Beta will have much less regressions and still allow you to try early features and fixes.  
They will give you the more stable experience that you are expecting.

Thanks to your report thought the regression was avoided for future builds within 4 hours of being reported.  Once the patch has been reviewed it will be pushed to Nightly.

Thanks again for your support.
Attachment #558304 - Flags: ui-review+
Attachment #558304 - Flags: superreview?
Attachment #558304 - Flags: review+
Attachment #558304 - Flags: feedback+
Attachment #558304 - Flags: checkin+
Attachment #558304 - Flags: approval1.9.2.21?
Attachment #558304 - Flags: approval1.9.2.20?
Attachment #558304 - Flags: approval-mozilla-release?
Attachment #558304 - Flags: approval-mozilla-beta?
Attachment #558304 - Flags: approval-mozilla-aurora?
Attachment #558304 - Attachment is obsolete: true
(In reply to Brian R. Bondy [:bbondy] from comment #13)
> Hi Peter & remixedcat.  
> 
> The answer was given in Comment 9 and Comment 5. 
> 
> Nightly builds are a wild place, they may or may not work.  
> Nightly builds are the place where testing happens, so regressions are
> common and expected.
> 
> Firefox Aurora or Firefox Beta will have much less regressions and still
> allow you to try early features and fixes.  
> They will give you the more stable experience that you are expecting.
> 
> Thanks to your report thought the regression was avoided for future builds
> within 4 hours of being reported.  Once the patch has been reviewed it will
> be pushed to Nightly.
> 
> Thanks again for your support.

I was talking about malware.
Ah, in that case Comment 13 minus Peter :)
Duplicate of this bug: 684756
Duplicate of this bug: 684795
Attachment #558094 - Flags: review?(jmathies) → review+
http://hg.mozilla.org/mozilla-central/rev/2b84c0ca82b6
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
This patch appears to still have major issues with the DLM. 

STR:
1.  Install a build with this patch 
2.  Open the Download Manager (ctrl+j) or via Menu 
3.  Right-click any file in the DM and select 'Open Folder Location' , browser crashes

There is no crash when using the old style manager, but its the wrong manager for Vista/Win7.  Why are we messing with 'custom' DM and not using the Native API.  Its worked for yrs until someone decided to 'fix' an ancient bug that no one was complaining about to beging with, so why not back out the whole thing and WONTFIX bug 295540 ?  For those with dual monitors, I rather imagine at this late date they have learned to just 'work-around' the problem as reported.

I don't have crash-reports as I'm testing with hourly builds.

Win7 64bit using m-c 32bit builds.

Regression range:
20110908135749 817c2b9dc11d crashes 
20110908132721 a73f7a3b85a5 Works, no crash when opening folder from DM
[quote]There is no crash when using the old style manager, but its the wrong manager for Vista/Win7.  Why are we messing with 'custom' DM and not using the Native API[/quote]

Wrong terminology,  should have said 'File picker'...
(In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #21)
> This patch appears to still have major issues with the DLM. 
> 
> STR:
> 1.  Install a build with this patch 
> 2.  Open the Download Manager (ctrl+j) or via Menu 
> 3.  Right-click any file in the DM and select 'Open Folder Location' ,
> browser crashes
> 
> There is no crash when using the old style manager, but its the wrong
> manager for Vista/Win7.  Why are we messing with 'custom' DM and not using
> the Native API.  Its worked for yrs until someone decided to 'fix' an
> ancient bug that no one was complaining about to beging with, so why not
> back out the whole thing and WONTFIX bug 295540 ?  For those with dual
> monitors, I rather imagine at this late date they have learned to just
> 'work-around' the problem as reported.
> 
> I don't have crash-reports as I'm testing with hourly builds.
> 
> Win7 64bit using m-c 32bit builds.
> 
> Regression range:
> 20110908135749 817c2b9dc11d crashes 
> 20110908132721 a73f7a3b85a5 Works, no crash when opening folder from DM

This crash is triggered by
0907fce1e6fc	Brian R. Bondy — Bug 670068 - Open Containing Folder spawns multiple instances of explorer.exe. r=jimm
Thanks Alice0775..
I haven't received a nightly update yet to test. Can someone who can reproduce this file a blocking on bug 670068 and post a crash signature plus the STR?
(In reply to Jim Mathies [:jimm] from comment #25)
> I haven't received a nightly update yet to test. Can someone who can
> reproduce this file a blocking on bug 670068 and post a crash signature plus
> the STR?

stack bp-e8f8e37f-9a3c-4e61-aed0-a3c592110909

STR: follow comment 21
(In reply to Jim Mathies [:jimm] from comment #25)
> I haven't received a nightly update yet to test. Can someone who can
> reproduce this file a blocking on bug 670068 and post a crash signature plus
> the STR?

https://crash-stats.mozilla.com/report/index/bp-54b73685-d4a7-49b4-8caf-e3d362110909

ref comment 26, that must be an hourly build your using, hourlys do not contain symbols to give an accurate stack-dump.

STR in comment 21

Filed: https://bugzilla.mozilla.org/show_bug.cgi?id=685847
> There is no crash when using the old style manager, but its the wrong manager for Vista/Win7.

I'm using Win7 and it seems like the correct file picker now on the Nightly build.

For the crash which I can reproduce with the nightly build, unrelated to this ticket, I will handle in Bug 670068
> I will handle in Bug 670068

... in Bug 685847 since it was just posted.
Now getting proper file picker here as well. Nice.
Status: RESOLVED → VERIFIED
Duplicate of this bug: 689732
Hi Brian, I've reported Bug 692469 and notified that my bug is duplicate of current one. Can you tell me when firefox stable version will get fixed (I mean ff7). It is very important, because we need multiple file upload dialog with win7 features (search, image preview, remember last state, etc). 

I'm a bit disappointed when see "target milestone: mozilla9" for this bug resolution. Does it means we have to wait till version 9 is out?!!
Yes it will be in version 9, which is a couple months away in December.
Duplicate of this bug: 693038
(In reply to Brian R. Bondy [:bbondy] from comment #33)
> Yes it will be in version 9, which is a couple months away in December.

Given this is a regression in 7, and the patch (afaict) is just adding some windows version checks, couldn't this be pushed forward to be fixed in version 8? Or do you see potential issues with it that need more testing?
Requesting beta+
 
This bug is a problem with the multi file picker using the older Windows file picker.
The problem is a regression that was introduced by the fix in bug 660833.
The problem was introduced and not noticed because Windows silently uses it whenever you use a hook on the filepicker dialog.

The problem is already fixed in Aurora and Nightly.
The problem is currently present in FF7 and Beta.
The regression first hit FF7 and was not in FF6.

Risk of introducing further regressions: None since we would be just backing out the most recent 2 changes in the file picker code of FF7.

2 changesets:
http://hg.mozilla.org/releases/mozilla-beta/rev/08a813a47709
http://hg.mozilla.org/releases/mozilla-beta/rev/84533f5e86bf

NOTE:****DO NOT use the patch as the code has changed too much to use it, instead backout the 2 changesets mentioned above.****
Attachment #566068 - Flags: approval-mozilla-beta?
Duplicate of this bug: 694757
Duplicate of this bug: 694853
Attachment #566068 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Onno, I'm resetting the flags to fixed for 8 as I assume you accidentally re-set them.
Yes, thanks Mark. Sorry for doing so.
You need to log in before you can comment on or make changes to this bug.