"Unknown error" occurs when trying to open a file with Across Lite from Firefox

RESOLVED FIXED in mozilla8

Status

()

Core
XPCOM
--
minor
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Adam Rosenfield, Assigned: Adam Rosenfield)

Tracking

unspecified
mozilla8
x86_64
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [inbound])

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

6 years ago
User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0) Gecko/20100101 Firefox/4.0
Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.2a1pre) Gecko/20110330 Firefox/4.2a1pre

When attempting to open a .puz file in Firefox using Across Lite, I get the following error dialog:

---------- snip ----------
<b>Download Error</b>

filename.puz could not be opened, because an unknown error occurred.

Try saving to disk first and then opening the file.
---------- snip ----------

Saving to disk and opening the file is successful, but that's rather annoying.

Reproducible: Always

Steps to Reproduce:
1. Download and install Across Lite software for Mac from <http://www.nytimes.com/downloads/acl2mac.dmg>
2. Click on a link to a .puz file, such as <http://icrossword.com/publish/server/puzzle/index.php/mgwcc_147.puz?id=BDDB_mgwcc_147.puz>
3. In the "What should Firefox do with this file?" dialog, click the "Choose..." button, navigate to the Across Lite v2.0 executable, and click Open
4. Click OK
Actual Results:  
The "unknown error" dialog is shown as described in the Details.  The .puz file fails to download or be opened in Across Lite.

Expected Results:  
The .puz file is downloaded and opened in Across Lite.

This bug appeared after upgrading from Firefox 3.6 to 4.0, with no changes to Across Lite.

This bug happens both when choosing Across Lite from the "What should Firefox do with this file?" dialog and when Across Lite is set as the default application for opening .puz files (which I'd had with FF 3.6).

This bug seems to be specific to Across Lite: this bug does not appear when trying to open .puz files with other applications (e.g. TextEdit).
(Assignee)

Comment 1

6 years ago
Created attachment 543290 [details] [diff] [review]
Proposed fix

Oh what the hell, I fixed it myself.  This bug was introduced by changeset 9c005e5d9719 in the fix for bug 571193.  Related to bug 663899 (though I don't have access to that bug).

This bug was occurring because Across Lite is a Mac Classic application with permissions of -rw-r--r--, so it was considered to be non-executable, even though it is actually executable according to Launch Services.
Attachment #543290 - Flags: review?(joshmoz)
Comment on attachment 543290 [details] [diff] [review]
Proposed fix

Josh moved to working on networking code fairly recently, redirecting...
Attachment #543290 - Flags: review?(joshmoz) → review?(smichaud)
Comment on attachment 543290 [details] [diff] [review]
Proposed fix

I'm busy with more urgent bugs, and it'll take me a while to fully
understand the implications of your patch (particularly of getting rid
of the special cases for *.jar and *.air files).

So it'll probably be a few weeks before I can get to this.

If it turns out I can't get to it within that time, I'll try to find
another reviewer.
(Assignee)

Comment 4

6 years ago
Ok.  I don't have access to bug 663899, so can't verify if there have been any regressions.  Was that bug for just Mac OS X, or was it occurring for all *nix flavors?  My patch was made on the assumption that that bug was only for OS X; if that's not the case, then the special case for .jar and .air files should be kept.
Bug 663899 is a security bug (which is why access is restricted).  So it's important to get these changes right :-)

Also (as I myself failed to notice earlier) the code you've changed *is* cross-platform (meant to be used with any Unix-style file system).  So at the very least any Mac-specific code would have to be ifdefed, and there's some question whether we should put any Mac-specific code there at all.

I need to wait until I'm not distracted by other things, and can devote my full attention to these problems.
Oh, and the fix for bug 663899 *is* also intended to be cross-platform.
(Assignee)

Comment 7

6 years ago
In that case, I need to fix my patch, which was based on an incorrect assumption.
(Assignee)

Comment 8

6 years ago
Created attachment 544394 [details] [diff] [review]
Proposed fix v2

This fixes this bug without affecting the behavior with regards to bug 663899; this patch affects only the Mac OS X build.
Attachment #543290 - Attachment is obsolete: true
Attachment #543290 - Flags: review?(smichaud)
Attachment #544394 - Flags: review?(smichaud)
(Assignee)

Comment 9

6 years ago
Created attachment 544395 [details] [diff] [review]
Proposed fix v2.1

Fixed typo in comment
Attachment #544394 - Attachment is obsolete: true
Attachment #544394 - Flags: review?(smichaud)
Attachment #544395 - Flags: review?(smichaud)
Created attachment 547483 [details] [diff] [review]
v 2.1 fix with narrowed scope

I've finally gotten time to work on this.

Your patch is basically fine, but I'd prefer something like this,
which makes a smaller change to existing code.  For example, it still
checks the executable bit if Launch Services doesn't identify
something as executable.
Oh, and since I'm able to reproduce this bug, I'll change the status to NEW.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 12

6 years ago
That seems sensible; I was copying the behavior pre-9c005e5d9719 (lines 1017-1040 of http://hg.mozilla.org/mozilla-central/diff/9c005e5d9719/xpcom/io/nsLocalFileOSX.mm), which just returned PR_FALSE if an error occurred.  I'm not sure what can cause GetCFURL or LSCopyItemInfoForRef to fail.
Comment on attachment 547483 [details] [diff] [review]
v 2.1 fix with narrowed scope

> I'm not sure what can cause GetCFURL or LSCopyItemInfoForRef to
> fail.

I don't know, either.  But I think it makes sense to treat a failure
in GetCFURL() more seriously than one in LSCopyItemInfoForRef(), as
both our patches do.

So if you're happy with my suggestions, the next steps are:

1) Create a copy of "my" patch (the one from comment #10) that has
your name as the developer, but which is easy for someone else to
land:

https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_chec\
k-in_for_me.3f

2) Add "checkin-needed" to the Keywords field.

   This will put the patch in the queue for being landed on the
   mozilla-inbound tree, which is periodically (about once a day)
   merged to the mozilla-central tree, which is in effect the "trunk".

Or if you'd prefer I can do this for you.
Attachment #547483 - Flags: review+
Attachment #544395 - Flags: review?(smichaud)
https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3f
(Assignee)

Comment 15

6 years ago
If it's easy for you to do, then go ahead.  Otherwise I'll take care of it when I get home to my Mac later tonight.
I'll wait for you to take care of it tonight.

If you haven't by tomorrow, I'll do it myself.
(Assignee)

Comment 17

6 years ago
Created attachment 547611 [details] [diff] [review]
Fix v3.0

Here goes; let me know if I didn't get the flags right.
Attachment #544395 - Attachment is obsolete: true
Attachment #547611 - Flags: review+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed

Updated

6 years ago
Assignee: nobody → adam
Component: File Handling → XPCOM
Product: Firefox → Core
QA Contact: file.handling → xpcom

Updated

6 years ago
Attachment #547483 - Attachment is obsolete: true
It all looks fine to me.

Thanks for your patch!

Updated

6 years ago
Keywords: checkin-needed
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/6632abc4dd0a
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.