Closed
Bug 1015115
Opened 11 years ago
Closed 11 years ago
Switch to cloneInto in PDFjs
Categories
(Firefox :: PDF Viewer, defect)
Firefox
PDF Viewer
Tracking
()
VERIFIED
FIXED
Firefox 32
People
(Reporter: Gijs, Assigned: Gijs)
References
Details
(Whiteboard: [https://github.com/mozilla/pdf.js/pull/4858] p=2 s=it-32c-31a-30b.3 [qa-])
Attachments
(1 file)
This is faster, more readable, and net code reduction. Patch coming up.
Flags: firefox-backlog+
Assignee | ||
Comment 1•11 years ago
|
||
This passes the m-bc tests as well as pdfjs' unit tests. I didn't run the reference tests because I don't think this change will have any effect on them.
Attachment #8427712 -
Flags: review?(ydelendik)
Updated•11 years ago
|
Whiteboard: https://github.com/mozilla/pdf.js/pull/4830
Comment 2•11 years ago
|
||
I actually have a similar patch to this code in bug 1015380. As far as I can tell, |detail| is a content object, so I don't understand why we need to expose it to content at all. Can someone enlighten me?
Assignee | ||
Comment 3•11 years ago
|
||
(In reply to Bobby Holley (:bholley) from comment #2)
> I actually have a similar patch to this code in bug 1015380. As far as I can
> tell, |detail| is a content object, so I don't understand why we need to
> expose it to content at all. Can someone enlighten me?
I would assume this to be the case for the sync case but not the async one?
Assignee | ||
Comment 4•11 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #3)
> (In reply to Bobby Holley (:bholley) from comment #2)
> > I actually have a similar patch to this code in bug 1015380. As far as I can
> > tell, |detail| is a content object, so I don't understand why we need to
> > expose it to content at all. Can someone enlighten me?
>
> I would assume this to be the case for the sync case but not the async one?
(I've updated the pull request to do the same as your patch from bug 1015380 for the sync case - this doesn't seem to break any of the mochitest-browser tests for pdfjs, so I presume it's OK.)
Assignee | ||
Comment 5•11 years ago
|
||
Marco, can you add this to the iteration backlog please?
Flags: needinfo?(mmucci)
Whiteboard: https://github.com/mozilla/pdf.js/pull/4830 → p=2 https://github.com/mozilla/pdf.js/pull/4858
Comment 6•11 years ago
|
||
Added to Iteration 32.3
Flags: needinfo?(mmucci)
Whiteboard: p=2 https://github.com/mozilla/pdf.js/pull/4858 → [https://github.com/mozilla/pdf.js/pull/4858] p=2 s=it-32c-31a-30b.3 [qa?]
Updated•11 years ago
|
Attachment #8427712 -
Flags: review?(ydelendik) → review+
Assignee | ||
Comment 7•11 years ago
|
||
Landed in bug 1017133.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•11 years ago
|
Status: RESOLVED → VERIFIED
Whiteboard: [https://github.com/mozilla/pdf.js/pull/4858] p=2 s=it-32c-31a-30b.3 [qa?] → [https://github.com/mozilla/pdf.js/pull/4858] p=2 s=it-32c-31a-30b.3 [qa-]
Updated•11 years ago
|
Target Milestone: --- → Firefox 32
You need to log in
before you can comment on or make changes to this bug.
Description
•