Nightly is using old file manager window

VERIFIED FIXED in Firefox 8

Status

()

Core
Widget: Win32
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: Terepin, Assigned: bbondy)

Tracking

({regression})

Trunk
mozilla9
All
Windows 7
regression
Points:
---

Firefox Tracking Flags

(firefox8 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
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.
(Reporter)

Updated

6 years ago
Keywords: regression
Hardware: x86_64 → All

Comment 1

6 years ago
Confirming on Windows 7 64-bit with the new 20110903 build.

Comment 2

6 years ago
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

Comment 3

6 years ago
Windows 7 32-bit
Nightly 20110903
Changeset: 76462:472716252ea3

Confirmed. Old-style file manager windows is shown.
(Assignee)

Updated

6 years ago
Assignee: nobody → netzen

Updated

6 years ago
Blocks: 295540

Updated

6 years ago
Duplicate of this bug: 684524
(Assignee)

Comment 5

6 years ago
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.
(Assignee)

Comment 6

6 years ago
Created attachment 558094 [details] [diff] [review]
Fixed Windows silently using old dialogs because of hook procedure.

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)
(Assignee)

Updated

6 years ago
Status: NEW → ASSIGNED

Updated

6 years ago
Duplicate of this bug: 684556

Comment 8

6 years ago
how did this even happen? it was fine before. What change really motivated this?
(Assignee)

Comment 9

6 years ago
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.

Comment 10

6 years ago
Created attachment 558304 [details]
Description
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?
(Reporter)

Comment 11

6 years ago
Serious question: How did this get uploaded?

Comment 12

6 years ago
(In reply to Peter Henkel [:Terepin] from comment #11)
> Serious question: How did this get uploaded?

my points exactly Terepin!!!
(Assignee)

Updated

6 years ago
Attachment #558304 - Attachment is patch: false
(Assignee)

Comment 13

6 years ago
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.
(Assignee)

Updated

6 years ago
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?
(Assignee)

Updated

6 years ago
Attachment #558304 - Attachment is obsolete: true
(Reporter)

Comment 14

6 years ago
(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.
(Assignee)

Comment 15

6 years ago
Ah, in that case Comment 13 minus Peter :)
Duplicate of this bug: 684756

Updated

6 years ago
Duplicate of this bug: 684795

Updated

6 years ago
Attachment #558094 - Flags: review?(jmathies) → review+
(Assignee)

Comment 18

6 years ago
Pushed to try:
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=4325bfb17f6f
(Assignee)

Comment 19

6 years ago
Pushed to mozilla-inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/2b84c0ca82b6
http://hg.mozilla.org/mozilla-central/rev/2b84c0ca82b6
Status: ASSIGNED → RESOLVED
Last Resolved: 6 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'...

Comment 23

6 years ago
(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..

Comment 25

6 years ago
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?

Comment 26

6 years ago
(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
(Assignee)

Comment 28

6 years ago
> 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
(Assignee)

Comment 29

6 years ago
> I will handle in Bug 670068

... in Bug 685847 since it was just posted.

Comment 30

6 years ago
Now getting proper file picker here as well. Nice.
(Reporter)

Updated

6 years ago
Status: RESOLVED → VERIFIED
(Assignee)

Updated

6 years ago
Duplicate of this bug: 689732

Comment 32

6 years ago
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?!!
(Assignee)

Comment 33

6 years ago
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?
tracking-firefox8: --- → ?
(Assignee)

Comment 36

6 years ago
Created attachment 566068 [details]
Request for reverting 2 changesets in Firefox Beta

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

Updated

6 years ago
Attachment #566068 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Christian did the backouts just before b4:

http://hg.mozilla.org/releases/mozilla-beta/rev/e3fb31311b62
http://hg.mozilla.org/releases/mozilla-beta/rev/75616c2e1ddf
status-firefox8: --- → fixed
tracking-firefox8: ? → ---
status-firefox8: fixed → ---
tracking-firefox8: --- → ?
Onno, I'm resetting the flags to fixed for 8 as I assume you accidentally re-set them.
status-firefox8: --- → fixed
tracking-firefox8: ? → ---
Yes, thanks Mark. Sorry for doing so.
You need to log in before you can comment on or make changes to this bug.