Closed Bug 914667 Opened 6 years ago Closed 6 years ago

PDF viewer doesn't resolve base URIs properly

Categories

(Firefox :: PDF Viewer, defect, P3)

x86
macOS
defect

Tracking

()

RESOLVED FIXED
Firefox 27
Tracking Status
firefox24 --- wontfix
firefox25 + verified
firefox26 + verified
firefox27 + verified
firefox-esr17 --- unaffected
firefox-esr24 25+ verified

People

(Reporter: gfritzsche, Assigned: yury)

References

Details

(Whiteboard: [pdfjs-c-integration][pdfjs-f-fixed-upstream] https://github.com/mozilla/pdf.js/pull/3735)

Attachments

(2 files)

With the standalone test-case from bug 238886 (attachment 802314 [details]), the PDF viewer seems to ignore the base URI or use an incorrect one.

Basically, for a page at http://localhost:8000/subdir/foo.html with:

<html><body>
<base href="http://localhost:8000/">
<embed src="1.pdf">

... Fx sends two requests:
http://localhost:8000/1.pdf (correct)
http://localhost:8000/subdir/1.pdf (incorrect & duplicate request)

It looks like the original channel gets opened here correctly, but is ignored, and then the PDF viewer initiates another request with an incorrect URL.
Priority: -- → P3
Whiteboard: [pdfjs-c-integration]
FWIW privileged code can read the object/embed tag's srcURI attribute to get the properly-resolved URI:

http://dxr.mozilla.org/mozilla-central/source/dom/webidl/HTMLObjectElement.webidl#l177
Status: NEW → RESOLVED
Closed: 6 years ago
Depends on: 922693
Resolution: --- → FIXED
Whiteboard: [pdfjs-c-integration] → [pdfjs-c-integration][pdfjs-f-fixed-upstream] https://github.com/mozilla/pdf.js/pull/3735
[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 738967
User impact if declined: pdf location not properly resolved
Testing completed (on m-c, etc.): present on m-c
Risk to taking this patch (and alternatives if risky): low, affects only pdf viewer when embedded using EMBED tag
String or IDL/UUID changes made by this patch: none
Attachment #818778 - Flags: review?(bdahl)
Attachment #818778 - Flags: approval-mozilla-beta?
[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 738967
User impact if declined: pdf location not properly resolved
Testing completed (on m-c, etc.): present on m-c
Risk to taking this patch (and alternatives if risky): low, affects only pdf viewer when embedded using EMBED tag
String or IDL/UUID changes made by this patch: none
Attachment #818779 - Flags: review?(bdahl)
Attachment #818779 - Flags: approval-mozilla-aurora?
Attachment #818778 - Flags: review?(bdahl) → review+
Attachment #818779 - Flags: review?(bdahl) → review+
Comment on attachment 818779 [details] [diff] [review]
bug914667-aurora.diff

Approving for Aurora.
Attachment #818779 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Keywords: checkin-needed
Whiteboard: [pdfjs-c-integration][pdfjs-f-fixed-upstream] https://github.com/mozilla/pdf.js/pull/3735 → checkin-needed for bug914667-aurora.diff [pdfjs-c-integration][pdfjs-f-fixed-upstream] https://github.com/mozilla/pdf.js/pull/3735
https://hg.mozilla.org/releases/mozilla-aurora/rev/1cb1ca523d16
Assignee: nobody → ydelendik
Keywords: checkin-needed
Whiteboard: checkin-needed for bug914667-aurora.diff [pdfjs-c-integration][pdfjs-f-fixed-upstream] https://github.com/mozilla/pdf.js/pull/3735 → [pdfjs-c-integration][pdfjs-f-fixed-upstream] https://github.com/mozilla/pdf.js/pull/3735
Target Milestone: --- → Firefox 27
Comment on attachment 818778 [details] [diff] [review]
bug914667-beta.diff

Approving for ESR too, if this can apply cleanly there - otherwise please prepare branch patch.
Attachment #818778 - Flags: approval-mozilla-esr24+
Attachment #818778 - Flags: approval-mozilla-beta?
Attachment #818778 - Flags: approval-mozilla-beta+
(In reply to lsblakk@mozilla.com [:lsblakk] from comment #6)

> Approving for ESR too, if this can apply cleanly there - otherwise please
> prepare branch patch.

esr24 contains same version of the file as beta branch
Keywords: checkin-needed
Whiteboard: [pdfjs-c-integration][pdfjs-f-fixed-upstream] https://github.com/mozilla/pdf.js/pull/3735 → checkin for bug914667-beta.diff for beta and esr24 branches [pdfjs-c-integration][pdfjs-f-fixed-upstream] https://github.com/mozilla/pdf.js/pull/3735
https://hg.mozilla.org/releases/mozilla-beta/rev/d8eb9996fe8e
https://hg.mozilla.org/releases/mozilla-esr24/rev/fd675d2d541f
Keywords: checkin-needed
Whiteboard: checkin for bug914667-beta.diff for beta and esr24 branches [pdfjs-c-integration][pdfjs-f-fixed-upstream] https://github.com/mozilla/pdf.js/pull/3735 → [pdfjs-c-integration][pdfjs-f-fixed-upstream] https://github.com/mozilla/pdf.js/pull/3735
Keywords: verifyme
I tested this fix using the testcase from Comment 0 on FF25b10, Mac OS 10.8.4

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:25.0) Gecko/20100101 Firefox/25.0
BuildID: 20131021191948

STR used:

Pre-requisites: Need python installed on your machine.

1. Download the testcase
2. Go in the test-case-directory in a shell/command-line
3. Run this command: python -m SimpleHTTPServer (now you can navigate to localhost)
4. In FF navigate to this URL: http://localhost:8000/1.pdf

Results: The PDF having "Hello Word" content is fully displayed without any duplicated request.

Based on my verification done I'm marking for now 25status as being Verified.
(In reply to Mihai Morar, QA (:MihaiMorar) from comment #9)

Same result for http://localhost:8000/subdir/index.html instead of Step 4 mentioned above.
QA Contact: mihai.morar
With regards to the incorrect URI resolution, I've verified the fix on branches 24esr, 26, 27, from 2013-10-21.

However, I see new behavior. While inspecting the network, I can see that Firefox now issues three requests for the PDF. I'd expect one. I can see this on all branches that have this patch.

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