The default bug view has changed. See this FAQ.

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

VERIFIED FIXED in Firefox 6

Status

()

Core
Widget: Cocoa
--
major
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: mounir)

Tracking

({regression, useless-UI, ux-implementation-level})

Trunk
mozilla7
All
Mac OS X
regression, useless-UI, ux-implementation-level
Points:
---
Bug Flags:
in-litmus ?

Firefox Tracking Flags

(firefox5 unaffected, firefox6+ fixed, firefox7 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Comment hidden (empty)
(Reporter)

Comment 1

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

Updated

6 years ago
status-firefox5: --- → unaffected
status-firefox6: --- → affected
tracking-firefox6: --- → ?
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?
Regression range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a45b5cf1d625&tochange=2a0fbd6eedbd

Looks like a regression from bug 656260.
Blocks: 656260
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.
(Assignee)

Comment 5

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

Updated

6 years ago
Hardware: x86_64 → All
(Assignee)

Updated

6 years ago
Component: File Handling → Widget: Cocoa
QA Contact: file-handling → cocoa
(Assignee)

Comment 6

6 years ago
Created attachment 537584 [details] [diff] [review]
Work around the preference
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?
(Assignee)

Comment 8

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

Comment 9

6 years ago
Created attachment 537590 [details] [diff] [review]
Remove the pref and fallback to 0

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

Comment 10

6 years ago
Created attachment 537591 [details] [diff] [review]
Remove the pref and fallback to 0

Sorry, that was the wrong file.
Attachment #537590 - Attachment is obsolete: true
Attachment #537590 - Flags: review?(joshmoz)
Attachment #537591 - Flags: review?(joshmoz)
(Assignee)

Updated

6 years ago
Whiteboard: [needs review]
tracking-firefox6: ? → +

Comment 11

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

Updated

6 years ago
Attachment #537591 - Flags: review?(joshmoz) → review+
(Assignee)

Updated

6 years ago
Flags: in-litmus?
Whiteboard: [needs review] → [inbound]
(Assignee)

Updated

6 years ago
status-firefox7: --- → affected
(Assignee)

Updated

6 years ago
Attachment #537591 - Flags: checkin+
(Assignee)

Comment 12

6 years ago
Pushed:
http://hg.mozilla.org/mozilla-central/rev/97b2a9eeab9c
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla7
(Assignee)

Updated

6 years ago
Attachment #537591 - Flags: approval-mozilla-aurora?
(Assignee)

Updated

6 years ago
status-firefox7: affected → fixed

Comment 13

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

Comment 14

6 years ago
Pushed to aurora:
http://hg.mozilla.org/releases/mozilla-aurora/rev/aad654481c86
status-firefox6: affected → fixed

Comment 15

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