801407 - PDF.js should be themeable.

Status: NEW

(Firefox :: PDF Viewer, enhancement, P3)

Description

[Alfred Kayser]

Currently, the toolbars and buttons of PDF.js cannot be themed, as they provide their own stylesheets outside chrome://*/skin/...
This makes it impossible to style these toolbars and buttons in the same style as the rest of the browser for custom themes (such as Walnut and Nautipolis).

Please, place the PDF.js stylesheets in chrome://pdfjs/skin/ so that we can style it.

Thanks!

Comment 1

[Alfred Kayser]

In short:
move viewer.css and corr. images to chrome://pdf.js/skin/
and change the reference resource://pdf.js/web/viewer.css to chrome://pdf.js/skin/viewer.css.

Comment 2

[Alfred Kayser]

Attachment: V1: Move the .css and images to chrome://pdf.js/skin/

Comment 3

[Yury Delendik (:yury)]

Instead of moving default resource://pdf.js/web/viewer.css create empty/default one in chrome://pdfjs/skin/ . That will avoid code duplication for different skins and let authors to "extend" stylesheet vs creating new one. (btw, main repo is located at https://github.com/mozilla/pdf.js)

Comment 4

[Alfred Kayser]

Attachment: V2: Use @import to allow skinning

Note, this solution is also used in for example aboutMemory.css.

Comment 5

[Alfred Kayser]

Attachment: V3: include the skin file with the import

Comment 6

[Alfred Kayser]

Attachment: V4: Oops wrong patch attacted, this is the right one

Comment 7

[Yury Delendik (:yury)]

Comment on attachment 717077 [details] [diff] [review]
V4: Oops wrong patch attacted, this is the right one

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

The files you are modifying are automatically generated using https://github.com/mozilla/pdf.js/blob/master/make.js#L434 script. Please submit your changes to the mozilla/pdf.js repo first.

::: browser/extensions/pdfjs/content/web/viewer.html
@@ +19,5 @@
>      <meta charset="utf-8">
>      <meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1">
>      <title>PDF.js viewer</title>
>  
> +    <!-- This snippet is used in firefox extension, see Makefile -->

This code was automatically inserted by build script. Also, this change is irrelevant to the issue.

@@ +27,2 @@
>  
> +    <link rel="stylesheet" href="chrome://pdf.js/skin/viewer.css"/>

The same. Keep it viewer.css

::: browser/extensions/pdfjs/extension-files
@@ +36,5 @@
>  content/web/images/toolbarButton-zoomOut.png
>  content/web/l10n.js
>  content/web/viewer.css
>  content/web/viewer.html
> +skin/viewer.css

Why you are removing viewer.js? This file is automatically generated

::: browser/extensions/pdfjs/skin/viewer.css
@@ +1,1 @@
> +@import url("resource://pdf.js/web/viewer.css");

Please add additional <link> tag to the https://github.com/mozilla/pdf.js/blob/master/web/viewer-snippet-firefox-extension.html (which will be included to viewer.html) and don't use @import.

Comment 8

[jlt0022]

Any updates on this?

Comment 9

[reinhart_previano]

Actually, the theme can be decided through CSS. I was be able to change the PDF.JS theme by modifying the CSS. The thing we need is to allow CSS editing through special directory. If this is happen, my custom PDF.JS theme would be installed on Firefox.

Comment 10

[Alfred Kayser]

(In reply to reinhart_previano from comment #9)
> Actually, the theme can be decided through CSS. I was be able to change the
> PDF.JS theme by modifying the CSS. The thing we need is to allow CSS editing
> through special directory. If this is happen, my custom PDF.JS theme would
> be installed on Firefox.

See comment #1.

As for comment #8: I get lost in the PDF.js repository on how to fix this.

Aparently, in https://github.com/mozilla/pdf.js/blob/master/web/viewer.html
one should replace viewer.css with something link chrome://pdfjs/skin/viewer.css,
and package this viewer.css and images so that chrome://pdfjs/skin can find it.

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:bdahl, could you have a look please?
For more information, please visit auto_nag documentation.