Closed Bug 662261 Opened 9 years ago Closed 9 years ago

[Mac] When trying to choose a helper app, the app bundle expands

Categories

(Core :: Widget: Cocoa, defect, major)

All
macOS
defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla7
Tracking Status
firefox5 --- unaffected
firefox6 + fixed
firefox7 --- fixed

People

(Reporter: jruderman, Assigned: mounir)

References

Details

(Keywords: regression, useless-UI, ux-implementation-level)

Attachments

(2 files, 1 obsolete file)

No description provided.
Steps to reproduce:
1. http://www.squarefree.com/bug662261/x.test
2. Click "Choose..."
3. Click TextEdit.app

Expected: the "Open" button should now be enabled

Result: the app bundle has been expanded, showing a "Contents" subfolder.


Regression: Bug is present on trunk (7) and aurora (6), but not beta (5).

Impact: Combined with bug 589176, this makes the "Open with helper app" feature entirely useless for people who don't know how to dig into an .app bundle and find the right executable. So somewhere between ux-implementation-level and useless-UI.

(And maybe the initial folder also stopped defaulting to /Applications/ like it should?)
So this is really weird....  I can reproduce in my normal browsing profile, but not in a clean profile using the same nightly.

Jesse, do you see this in a clean profile too?
The pref that causes the problem for me is "filepicker.lastTypeIndex", which is set to "1".

This is set because at some point I saved a web page and chose "HTML only".

And then nsFilePicker::GetFilterList() is just broken:

548   if (mFilters.Length() <= (PRUint32)mSelectedTypeIndex) {
549     return nil;
550   }

so if mFilters.Length() == 1 in my case (as it is for the app picker, which just has the filterApps filter), GetFilterList will incorrectly return nil instead of the actual single filter desired.  And then |filters| is null in the code bug 656260 added, and things break.
This bug actually appeared with bug 643576 but was hidden because of a very old bug fixed by bug 656260.

I'm going to attach a patch that fixes the issue but I would prefer to just remove "filepicker.lastTypeIndex" preference which is only used on MacOS and doesn't seem useful: you might want to select the first item of a filter list sometimes and the second some other times and the preference will try to default to the wrong one each time...
Josh, would you be okay if I remove this 'feature'?
Blocks: 643576
No longer blocks: 656260
Hardware: x86_64 → All
Component: File Handling → Widget: Cocoa
QA Contact: file-handling → cocoa
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Attachment #537584 - Flags: review?(joshmoz)
So what does this preference do?  Persist the last-selected filter index?

For the specific case of "html only" vs "complete", do we remember the chosen value elsewhere?
(In reply to comment #7)
> So what does this preference do?  Persist the last-selected filter index?
> 
> For the specific case of "html only" vs "complete", do we remember the
> chosen value elsewhere?

Yes, but I think it's done by toolkit/content/contentAreaUtils.js

Except if this code doesn't work for MacOS X, it looks like we do that twice.
I prefer this patch that removes the cause of the failure and also falls back to 0 (and warns) when the index is out of range. I believe it's a saner fallback than fallback to no filter.
Attachment #537590 - Flags: review?(joshmoz)
Sorry, that was the wrong file.
Attachment #537590 - Attachment is obsolete: true
Attachment #537590 - Flags: review?(joshmoz)
Attachment #537591 - Flags: review?(joshmoz)
Whiteboard: [needs review]
Comment on attachment 537584 [details] [diff] [review]
Work around the preference

Canceling review here because it seems like you prefer the other patch and I do too.
Attachment #537584 - Flags: review?(joshmoz)
Attachment #537591 - Flags: review?(joshmoz) → review+
Flags: in-litmus?
Whiteboard: [needs review] → [inbound]
Attachment #537591 - Flags: checkin+
Pushed:
http://hg.mozilla.org/mozilla-central/rev/97b2a9eeab9c
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla7
Attachment #537591 - Flags: approval-mozilla-aurora?
Comment on attachment 537591 [details] [diff] [review]
Remove the pref and fallback to 0

Approved for mozilla-aurora
Attachment #537591 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Setting resolution  to Verified Fixed on Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:6.0) Gecko/20100101 Firefox/6.0 beta 5

The open button is enabled.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.