Last Comment Bug 777388 - PDF Viewer settings not persistent
: PDF Viewer settings not persistent
Status: RESOLVED FIXED
[testday-20121101]
:
Product: Firefox
Classification: Client Software
Component: PDF Viewer (show other bugs)
: 15 Branch
: All All
: -- normal (vote)
: Firefox 17
Assigned To: Yury Delendik (:yury)
:
Mentors:
: 768444 775214 777390 777407 (view as bug list)
Depends on: 790643 741517 787820
Blocks: 748923
  Show dependency treegraph
 
Reported: 2012-07-25 09:47 PDT by oscarguit
Modified: 2012-11-01 15:36 PDT (History)
12 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Prevents updating the settings when internal handler is chosen (2.32 KB, patch)
2012-08-10 11:41 PDT, Yury Delendik (:yury)
mak77: review+
Details | Diff | Splinter Review
patch for checkin (2.28 KB, patch)
2012-08-20 12:00 PDT, Yury Delendik (:yury)
no flags Details | Diff | Splinter Review

Description oscarguit 2012-07-25 09:47:47 PDT
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:15.0) Gecko/20100101 Firefox/15.0
Build ID: 20120717110313

Steps to reproduce:

Trying to open several PDF files with PDF Viewer in Firefox 15b1, Mac OS X Lion 10.7.4


Actual results:

Some pdf files are not opened with PDF Preview 

Steps to reproduce the problem:

1. Check Preferences, Applications, if you search for PDF you have two entries with these settings:
   a. Portable Document Format (application/x-pdf) - Ask always
   b. Portable Document Format (PDF) - Preview in Firefox

2. Go to http://www.obets.ua.es/obets/libros/LibroReflexiones.pdf, it will open with PDF Viewer.

3. Go to http://www.aneca.es/content/download/10522/118049/file/academia_guiadeayuda_110310.pdf

4. Step 3 will not use PDF Viewer but t will show you the download window, with "Save file" and "Do it automatically for this type of archive" checked by default. Click Ok.

5. Now repeat steps 2 and 3. In both cases, Firefox will download the pdfs and PDF Viewer will not be used anymore. In Preferences, Applications you will find that settings for PDF have changed:
   a. Portable Document Format (application/x-pdf) - Ask always
   b. Portable Document Format (PDF) - Save file



Expected results:

I don't know the difference in the files from steps 2 and 3, but it seems to be treated as different file types. But in any case the action in one file type (download) should not affect the other file type (PDF Viewer)...
Comment 1 Brendan Dahl [:bdahl] 2012-08-02 12:56:19 PDT
I can confirm. Further more, even if you uncheck the "Do it automatically for this type of archive" it still changes the content handler.
Comment 2 Brendan Dahl [:bdahl] 2012-08-02 12:57:32 PDT
*** Bug 777407 has been marked as a duplicate of this bug. ***
Comment 3 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-08-06 14:50:19 PDT
This seems like a bug that should block us shipping pdf.js. Is there a tracking bug for that?
Comment 4 oscarguit 2012-08-09 11:08:31 PDT
Just in case this information is useful, this problem was not present when I was using PDF Viewer as an add-on in FF 14.
Comment 5 Yury Delendik (:yury) 2012-08-10 11:41:59 PDT
Created attachment 650961 [details] [diff] [review]
Prevents updating the settings when internal handler is chosen
Comment 6 Yury Delendik (:yury) 2012-08-10 12:00:18 PDT
*** Bug 768444 has been marked as a duplicate of this bug. ***
Comment 7 Yury Delendik (:yury) 2012-08-10 12:30:48 PDT
Comment on attachment 650961 [details] [diff] [review]
Prevents updating the settings when internal handler is chosen

Tentatively assigning the reviewer based on the familiarity with Bug 752676.
Comment 8 Marco Bonardo [::mak] 2012-08-20 07:34:16 PDT
Comment on attachment 650961 [details] [diff] [review]
Prevents updating the settings when internal handler is chosen

Review of attachment 650961 [details] [diff] [review]:
-----------------------------------------------------------------

I tried to follow all of the code paths here, and it looks correct, the comment is clear enough.
So basically, an internal handler may fallback to a download dialog, but we should not remember the download action in such a case since it's not user's action.  Though if at this point the user activates the checkbox we still save the action.  This makes sense to me.

Now, I'd probably ask for a test, but I'm not sure how much that may be time-consuming, considered it involves dialogs and adding a test-only handler or assuming pdf.js is available.  Plus considering the code is a quite straight early-return, it should be strong enough against goofy changes.  I'd say to just proceed with the fix, that looks quite important for proper functionality of pdf.js.
Comment 9 Yury Delendik (:yury) 2012-08-20 12:00:27 PDT
Created attachment 653464 [details] [diff] [review]
patch for checkin

try server https://tbpl.mozilla.org/?tree=Try&rev=2e2598a4b5d4
Comment 10 Yury Delendik (:yury) 2012-08-20 12:03:36 PDT
*** Bug 775214 has been marked as a duplicate of this bug. ***
Comment 11 Ryan VanderMeulen [:RyanVM] 2012-08-20 17:22:43 PDT
(In reply to Yury (:yury) from comment #9)
> try server https://tbpl.mozilla.org/?tree=Try&rev=2e2598a4b5d4

Green on Try.

https://hg.mozilla.org/integration/mozilla-inbound/rev/17ddc4d169e1
Comment 12 Ed Morley [:emorley] 2012-08-21 06:29:34 PDT
https://hg.mozilla.org/mozilla-central/rev/17ddc4d169e1
Comment 13 JK 2012-08-23 07:33:30 PDT
*** Bug 777390 has been marked as a duplicate of this bug. ***
Comment 14 Gabriela [:gaby2300] 2012-11-01 15:36:10 PDT
Verified fixed using Windows 7 and the latest Nightly.

Note You need to log in before you can comment on or make changes to this bug.