Add project wordmark to print styles
Closed, ResolvedPublic3 Estimated Story Points
Actions

Assigned To

Authored By

	• Nirzar
	Jul 5 2017, 11:08 PM

Description

In order to raise awareness of our projects, we need to include the project wordmark to the the print styles of articles

Design

Zeplin Spec
https://zpl.io/26T0fw

Where does the logo come from?
We recently added project wordmarks to mobilefrontend headers, We would like the same project wordmark to appear here.

Developer notes

This is going to be a little tricky. We need to include an entry in a configurable variable ($wgMinervaCustomLogos) inside a css file.

The ResourceLoaderSkinModule.php does exactly this. Options are subclassing ResourceLoaderFileModule or registering a LESS variable inside onResourceLoaderGetLessVars (see @matmarex's super useful comments > T169826#3415344)

Test plan

Visit https://reading-web-staging.wmflabs.org (desktop skin)
Visit any page and click browser print function (Ctrl+P)
Expected: There should be a Wikipedia logo at the top of the page

Details

Subject	Repo	Branch	Lines +/-
Change print wordmark margin bottom to 20px from 10px	mediawiki/skins/Vector	master	+1 -1
Embed print wordmark and pre-render it	mediawiki/skins/Vector	master	+30 -12
Add print logo	mediawiki/skins/Vector	master	+102 -2

Customize query in gerrit

Related Objects
Search...

Status	Subtype	Assigned	Task
Resolved		• JKatzWMF	T150871 [EPIC] (Proposal) Replicate core OCG features and sunset OCG service
Resolved		TheDJ	T150872 Replace OCG in collection extension with Electron
Duplicate		None	T150874 Collate wikimedia pages into a single html wikimedia page that can then be rendered into a single pdf
Resolved		• JKatzWMF	T150875 Confirm attribution needs
Declined		None	T150917 Remove deprecated features from book creator UI
Resolved		pmiazga	T163272 [Spike] Determine changes necessary for concatenation support
Invalid		None	T167210 [EPIC] Adding PDF TOC with PDF page numbers to electron
Resolved	Spike	phuedx	T168004 [Spike 6hrs] Investigate ability of vivliostyle to render single articles
Invalid		None	T169757 Improve usability of "download as pdf" page
Invalid		None	T186740 [EPIC] It should be possible to print a book using the Proton service
Invalid	Spike	None	T169738 [Spike 8hrs] Investigate ability of using post-processing approach with new print styles
Resolved		Jdlrobson	T154965 [EPIC] Print styles - desktop
Duplicate		None	T169729 Implement new print styles for desktop
Resolved		• Nirzar	T169823 Design changes to desktop print styles
Resolved		• Nirzar	T169826 Add project wordmark to print styles
Declined		Jdlrobson	T171114 Show important indicators in print styles
Declined		None	T171250 Display categories while printing articles again
Resolved		• Nirzar	T171330 Justify text in print styles?
Resolved		• Nirzar	T172144 References take up more space with new print styles than existing print style
Resolved		• Nirzar	T173767 reduce space taken by TOC in new print styles
Resolved		ovasileva	T179363 Remove tocnumbers from TOC layout in print mode as they display incorrectly with numbers > 10 and their usefulness is debatable

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

ovasileva moved this task from 2016-17 Q4 to 2017-18 Q1 on the Web-Team-Backlog board.Jul 13 2017, 5:40 PM

• Tbayer unsubscribed.Jul 14 2017, 10:59 PM

thanks @matmarex that's super helpful advice.

If someone wants to work on this feel free to base off of https://gerrit.wikimedia.org/r/366173

• bmansurov claimed this task.Jul 19 2017, 5:14 PM

• bmansurov moved this task from Needs Design Review to Doing on the Readers-Web-Kanbanana-Board-Old board.

Change 366315 had a related patch set uploaded (by Bmansurov; owner: Bmansurov):
[mediawiki/skins/Vector@master] Add print logo

https://gerrit.wikimedia.org/r/366315

gerritbot added a project: Patch-For-Review.Jul 19 2017, 7:35 PM

• bmansurov moved this task from Doing to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.Jul 19 2017, 9:07 PM

• Nirzar mentioned this in T171114: Show important indicators in print styles.Jul 19 2017, 9:59 PM

• Nirzar updated the task description. (Show Details)

In T169826#3415344, @matmarex wrote:

The ResourceLoaderSkinModule.php does exactly this, but it's not done in a very generic way. We'll either have to extend this or adjust the implementation in core.

A generic way to generate CSS depending on config options is to subclass ResourceLoaderFileModule and override the getLessVars() method. You can then write your styles as Less like you normally would, and use the extra magically generated variables in it. See ResourceLoaderEditToolbarModule for an example.

Indeed. Per T140804, do not introduce new usage of this hook.

@bmansurov sincere apologies for change in position of wordmark mid development. a use-case was unearthed while developing T169823 and a follow-up task has been filed for it here T171114 which impacts this task in terms of the position of wordmark.

I try to avoid such things but I think I dropped the ball on this one.

Zeplin has been updated.

change requested: moving wordmark from right hand side to left hand side above the title of the article. similar to our mobile print styles

• bmansurov moved this task from Needs Code Review to Doing on the Readers-Web-Kanbanana-Board-Old board.Jul 20 2017, 12:58 PM

@Nirzar np. I also noticed that the mocks don't take long multi-lined titles into account. Although that's not a problem with the new watermark location change, you may want to add new mocks to show how indicators should be positioned when the title spans multiple lines. Here is an example title.

Jdlrobson mentioned this in T169823: Design changes to desktop print styles .Jul 20 2017, 6:53 PM

matmarex unsubscribed.Jul 22 2017, 7:49 PM

The icons on right will be bottom aligned with the border bottom of H1

wrapping text around it would be wonderful

• bmansurov moved this task from Doing to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.Jul 24 2017, 10:53 PM

@Nirzar are we ok not showing the wordmark for PDFs printed via IE8? If we use SVG's it will make the configuration simple, but IE8 won't be able to handle it. Otherwise, we can use PNG's but have a little complex configuration file.

are we ok not showing the wordmark for PDFs printed via IE8?

yeah.. we can drop wordmark support for IE8

Just to make sure, for IE8 then, we'll be displaying text?

No, broken image placeholder, something like T166684.

@bmansurov T

That's not what I'm seeing. Your current implementation shows no image for IE8. Not a broken image placeholder.

That's right. I just realized that.

Jdlrobson mentioned this in T169730: Define and implement instrumentation for printing on desktop web.Aug 1 2017, 6:57 PM

Change 366315 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Add print logo

https://gerrit.wikimedia.org/r/366315

ReleaseTaggerBot added a project: MW-1.30-release-notes (WMF-deploy-2017-08-08_(1.30.0-wmf.13)).Aug 2 2017, 1:00 AM

Jdlrobson moved this task from Needs Code Review to Ready for Signoff on the Readers-Web-Kanbanana-Board-Old board.Aug 2 2017, 1:32 AM

• bmansurov removed • bmansurov as the assignee of this task.Aug 2 2017, 2:05 AM

• bmansurov added a subscriber: matmarex.

matmarex unsubscribed.Aug 2 2017, 5:56 PM

when clicking print the wordmark sometimes doesn't appear on the printed page. Baha also said that the image width/height are showing up as 0. Let's debug why.

• bmansurov claimed this task.Aug 3 2017, 7:34 PM

• bmansurov moved this task from Needs More Work to Doing on the Readers-Web-Kanbanana-Board-Old board.

• bmansurov added a subscriber: matmarex.

when clicking print the wordmark sometimes doesn't appear on the printed page.

@Nirzar I remember you telling that this was the case. Can you share your browser info and the steps to reproduce this? Is the logo not visible in preview mode or in the final PDF?

OK, turns out there were two configuration options in for the same configuration variable in different places. I've removed the one in LocalSettings.php and kept the one in 10-Vector.php.

• bmansurov removed • bmansurov as the assignee of this task.Aug 3 2017, 7:59 PM

• bmansurov moved this task from Doing to Ready for Signoff on the Readers-Web-Kanbanana-Board-Old board.

. I've removed the one in LocalSettings.php and kept the one in 10-Vector.php.

Did you mean the opposite? The latter is written every time vagrant updates. I'm seeing the rule in:
/srv/mediawiki-vagrant/LocalSettings.php

I'm still not seeing the logo on several pages as reported by @Nirzar and @ovasileva so I will take some time to investigate that.

I created 10-Vector.php inside settings.d and I don't think vagrant touches those files. The config is in that file. Not sure why you see it in LocalSettings.php.

jdlrobson@reading-web-staging-3:~$ sudo su mwvagrant
mwvagrant@reading-web-staging-3:~$ cd /srv/mediawiki-vagrant/
mwvagrant@reading-web-staging-3:/srv/mediawiki-vagrant$ ag MinervaCustomLogos
LocalSettings.php
165:$wgMinervaCustomLogos = [

I see. Look for $wgVectorPrintLogo.

Got it. But yes those should be /srv/mediawiki-vagrant/LocalSettings.php

/vagrant/settings.d/*.php is for your own custom settings files. (As indicated README, and 00-debug.php-example). The subdirectory /vagrant/settings.d/puppet-managed/ is indeed managed by puppet. There, any modifications or extra files will be overwritten.

The same also applies per-wiki. /vagrant/settings.d/wikis/$mywiki/*.php and /vagrant/settings.d/wikis/$mywiki/settings.d/puppet-managed/*.php are managed by puppet. And any files matching /vagrant/settings.d/wikis/$mywiki/settings.d/*.php may be your own.

Personally, I limit my custom settings to just this one single file /vagrant/settings.d/10-mystuff.php. I leave everything else alone (including no changes to root LocalSettings.php, because it already contains various defaults and is thus harder to maintain).

Thanks for clarifying this @Krinkle is there a wiki page I can add to team's bookmarks which explains this?

At the top of /srv/mediawiki-vagrant/LocalSettings.php I see:

* To customize your MediaWiki instance, you may change the content of this
* file. See settings.d/README for an alternate way of managing small snippets
* of configuration data, such as extension invocations.

Sound like this is misleading?

Jdlrobson moved this task from Doing to Needs More Work on the Readers-Web-Kanbanana-Board-Old board.Aug 3 2017, 10:11 PM

Okay.. I've worked out the problem here. It's very similar to lazy loading images.
The problem here, is that the background image will only get loaded when the print function is async.

As I mentioned in the spike T171162 the print function is synchronous.

This is what I see happening in Chrome:

Visit http://reading-web-staging.wmflabs.org/wiki/William_Cooley and click native browser function
View preview (Chrome) - wordmark is not visible
Close dialog
Click print again
Wordmark is visible

The issue is because:

Clicking print is synchronous action. In background image is loaded, but when loaded the preview does not get refreshed.

Possible solutions:

Apply the css on all page views (don't restrict to print media query), but use display: none to hide in non-print mode.
Add a JS event handler that hooks into the onbeforeprint action to load the image (untested - might not work)
Load the image into the browser cache via a call to new Image( that occurs inside a setTimeout.

Jdlrobson removed a project: Patch-For-Review.Aug 3 2017, 10:16 PM

Nikerabbit subscribed.Aug 4 2017, 9:25 AM

matmarex updated the task description. (Show Details)Aug 4 2017, 11:24 AM

matmarex unsubscribed.

• bmansurov claimed this task.Aug 4 2017, 2:11 PM

• bmansurov moved this task from Needs More Work to Doing on the Readers-Web-Kanbanana-Board-Old board.

@bmansurov how are you planning to solve this? Would be good to talk through the options!

@Jdlrobson thanks for looking into the issue.

Apply the css on all page views (don't restrict to print media query), but use display: none to hide in non-print mode.

The browser would still not load the image because it's hidden by default, would it?

The other two options you mention require JavaScript, so I think we should look for another alternative. Maybe use data-URI's.

Change 370221 had a related patch set uploaded (by Bmansurov; owner: Bmansurov):
[mediawiki/skins/Vector@master] Embed print wordmark

https://gerrit.wikimedia.org/r/370221

gerritbot added a project: Patch-For-Review.Aug 4 2017, 3:10 PM

I've submitted a patch to embed the logo, but that does still not work on Firefox for some reason. It only works when generating a print preview, but the wordmark is still invisible when a PDF is printed directly (without generating a preview).

• bmansurov moved this task from Doing to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.Aug 7 2017, 1:47 PM

Jdlrobson moved this task from Needs Code Review to Needs QA on the Readers-Web-Kanbanana-Board-Old board.Aug 7 2017, 4:57 PM

Change 370221 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Embed print wordmark and pre-render it

https://gerrit.wikimedia.org/r/370221

Jdlrobson reassigned this task from • bmansurov to ABorbaWMF.Aug 7 2017, 5:00 PM

Jdlrobson removed a project: Patch-For-Review.

Jdlrobson updated the task description. (Show Details)

Over to you @ABorbaWMF

@pmiazga, sorry for the delay here. I am seeing the same behavior that @Jdlrobson reported above when I try it on chrome.

"This is what I see happening in Chrome:

Visit http://reading-web-staging.wmflabs.org/wiki/William_Cooley and click native browser function
View preview (Chrome) - wordmark is not visible
Close dialog
Click print again
Wordmark is visible"

So far it looks good on Safari and Edge.

Please can you look again @ABorbaWMF looks like some old code was running on staging.

Yep, looks good now on Chrome too

The space between the wordmark and the H1 title is not correct

https://zpl.io/VQMYAAV

spec here ^

The spec says 18.1px. Surely, it's not right. What should the value be, @Nirzar?

Jdlrobson moved this task from Ready for Signoff to Needs More Work on the Readers-Web-Kanbanana-Board-Old board.Aug 22 2017, 7:25 PM

Oh it says 18.1 because of line height for H1, zeplin seems to take wrong viewbox for the H1 I will report a bug. the correct value is 20px.

Sorry about that

• bmansurov claimed this task.Aug 22 2017, 8:10 PM

• bmansurov moved this task from Needs More Work to Doing on the Readers-Web-Kanbanana-Board-Old board.

• bmansurov added a subscriber: ABorbaWMF.

Change 373141 had a related patch set uploaded (by Bmansurov; owner: Bmansurov):
[mediawiki/skins/Vector@master] Change print wordmark margin bottom to 20px from 10px

https://gerrit.wikimedia.org/r/373141

gerritbot added a project: Patch-For-Review.Aug 22 2017, 8:26 PM

• bmansurov moved this task from Doing to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.Aug 22 2017, 8:26 PM

thanks @bmansurov !

Jdlrobson moved this task from Needs Code Review to Ready for Signoff on the Readers-Web-Kanbanana-Board-Old board.Aug 22 2017, 10:14 PM

Change 373141 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Change print wordmark margin bottom to 20px from 10px

https://gerrit.wikimedia.org/r/373141

ReleaseTaggerBot edited projects, added MW-1.30-release-notes (WMF-deploy-2017-08-29 (1.30.0-wmf.16)); removed MW-1.30-release-notes (WMF-deploy-2017-08-08_(1.30.0-wmf.13)).Aug 22 2017, 11:00 PM

back to you for sign off. It's been updated on staging.

Macro votecat: looks good

Jdlrobson mentioned this in T259197: Print stylesheet alternate logo downloads on all page views.Jul 29 2020, 10:23 PM

	F8804524: Berlin Copy.png
	Jul 24 2017, 5:40 PM

	F8804525: Berlin Copy 3.png
	Jul 24 2017, 5:40 PM

	F8799421: image.png
	Jul 19 2017, 10:00 PM

Add project wordmark to print stylesClosed, ResolvedPublic3 Estimated Story PointsActions

Description

Developer notes

Test plan

Details

Related ObjectsSearch...

Event Timeline

Add project wordmark to print styles
Closed, ResolvedPublic3 Estimated Story Points
Actions

Related Objects
Search...