796584 - Don't use localStorage in pdfjs

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[GH to BZ]

[GitHub issue by fabricedesre on 2012-08-31T14:29:27Z, https://github.com/mozilla-b2g/gaia/issues/4224]
pdfjs/content/web/viewer.js:155:// First we see if localStorage is available
pdfjs/content/web/viewer.js:158:  var isLocalStorageEnabled = (function localStorageEnabledTest() {
pdfjs/content/web/viewer.js:160:    // The additional localStorage call is to get around a FF quirk, see
pdfjs/content/web/viewer.js:163:      return 'localStorage' in window && window['localStorage'] !== null &&
pdfjs/content/web/viewer.js:164:          localStorage;
pdfjs/content/web/viewer.js:178:      database = localStorage.getItem('database') || '{}';

Comment 1

[GH to BZ]

[GitHub comment by timdream on 2012-09-11T17:04:50Z]
Adding `good first bug`. Anyone interested can replace the calls with @davidflanagan's asyncStorage.

Comment 2

[GH to BZ]

[GitHub comment by SKatiyar on 2012-09-18T17:51:30Z]
@fabricedesre, I am interested in taking up this bug.

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Will work on this as there seems to be no activity from SKatiyar.

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

Attachment: replace localStorage with asyncStorage

Comment 5

[Brendan Dahl [:bdahl]]

It would be ideal if this was a pull request against our github repo at https://github.com/mozilla/pdf.js/pulls . We currently treat mozcentral as a downstream project.

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

Yeah I thought about the pull request, will do. I wonder, is this really an upstream patch? That's not really something that touches the PDF.JS library but just the viewer that accesses the API. Also asyncStorage is custom to B2G.

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/5629

Pointer to Github pull-request

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

Attachment: Pointer to Github pull request: https://github.com/mozilla/pdf.js/pull/2186

Pointer to Github pull-request

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/5638

Pointer to Github pull-request

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 667236 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/5638

Hey Brendan, is this ready to be merged to master? Following the workflow suggested by Vivien I'm going to ask for review here and then change the commit message accordingly.

Comment 11

[Alex Keybl [:akeybl]]

What's the user impact if we don't take this fix?

Comment 12

[Brendan Dahl [:bdahl]]

(In reply to Alex Keybl [:akeybl] from comment #11)
> What's the user impact if we don't take this fix?

The scroll location of revisiting pdfs won't be remembered.


As for review, I had left a comment in the github bug, I didn't realize I needed to r+ here as well.

Comment 13

[Tim Taubert [:ttaubert] (inactive)]

https://github.com/mozilla-b2g/gaia/commit/a84ae1934604557b8d9ff8ba1838d3173e8290d1