hdpi/xhdpi resolution assets and layout for PDF Viewer app

VERIFIED FIXED

Status

VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: timdream, Assigned: rudyl)

Tracking

unspecified
x86
Mac OS X

Firefox Tracking Flags

(blocking-b2g:hd+, b2g-v1.1hd fixed)

Details

(Whiteboard: [GAIA_HD_ASSETS])

Attachments

(4 attachments)

This bug represents fixes needed in PDF Viewer app in order to deliver bug 881126:

-- Have visual supply us with @1.5x and the correct @2x images.
-- On v1.1hd, modify CSS/JS so that apps will display @1.5x/@2x images correctly.
-- On master, drop the @1.5x and @2x images and make sure there is no
more to tweak besides changes made on bug 830644.

As HIDPI support never landed on v1-train, I expect the patch for both
branches on this bug will be different. The CSS/JS patch on v1.1hd will be based on the patch on master, but I won't expect it to be as simple as a |git cherry-pick|; so this is
where the engineering work lays.

The patch on top of v1-train should land on v1.1hd.
Setting hd+ for 1.1hd specific requirement
blocking-b2g: hd? → hd+
This should be fairly simple.
Assignee: nobody → rlu
According to https://github.com/mozilla/pdf.js/pull/3269#issuecomment-20631691,
the assets and maybe CSS tweaks needed for this bug would be complete with Bug 876906.

However, I don't see why Bug 876906 is related to hi-resolution support.

Hi bdahl, 
May I know if we have an estimate when the assets would be done?
Thanks.
Flags: needinfo?(bdahl)
The bug is not related, but from that bug we will update pdf.js on gaia which will include hidpi support from https://github.com/mozilla/pdf.js/pull/3269.  

Just to be clear there are still some images missing and I will not be providing those as I did not create them.
Flags: needinfo?(bdahl)
Hi Brendan,

Thanks for the clarification.

Hi Patryk,

Are you the one we should consult to get the 2x/1.5x images for pdf.js or we should contact Eric Pang directly?

The assets we need are here,
https://github.com/mozilla/pdf.js/tree/master/extensions/b2g/images

- icon_next_page.png 	
- icon_previous_page.png 	
- icon_zoom_in.png 	
- icon_zoom_out.png 	
- spinner.png

Thanks.
Flags: needinfo?(padamczyk)
Ping Eric as well.

Eric, can you help to provide the assets we need as Comment 5?
Thanks
Flags: needinfo?(epang)
Amy will be able to provide graphics for this.
Flags: needinfo?(padamczyk)
Flags: needinfo?(epang)
Flags: needinfo?(amylee.design)

Comment 8

5 years ago
Created attachment 774027 [details]
Requested graphics

Hi, 

Attached are the requested graphics. We don't have the vector of "spinner.png" since it was created by Telefonica. A request was sent for the graphic so I will attach it when I get it.
Flags: needinfo?(amylee.design)
Hi Amy,

Could you also provide the following graphics,

 1. @2x
   - icon_next_page.png 	
   - icon_previous_page.png 	   
   - icon_zoom_in.png 	
   - icon_zoom_out.png 	
   - spinner.png

 2. @1.5x
   - div_line_left.png
   - div_line_right.png
   - spinner.png

Thank you.
Flags: needinfo?(amylee.design)
Created attachment 774613 [details]
PDF Assets

Hi Rudy, 

Attached are the right/left div lines. I am waiting for the "spinner.png" graphic from Telefonica so I will send it over when I get it. I don't think we are supporting @2x graphic sizes. Patryk can you confirm?
Attachment #774613 - Flags: feedback?(padamczyk)
Flags: needinfo?(amylee.design)
Created attachment 774655 [details]
Progress_Activity_Assets@1.5

Hi Rudy, 

Telefonica got back to me and they said that the "spinner.png" is out of date and should not be used. I've attached what we are using for the spinner graphic and here is a link to the guidelines. 

https://wiki.mozilla.org/Gaia/Design/BuildingBlocks#Progress_.26_Activity_Indicators
(In reply to Amy from comment #10)
> Created attachment 774613 [details]
> PDF Assets
> 
> Hi Rudy, 
> 
> Attached are the right/left div lines. I am waiting for the "spinner.png"
> graphic from Telefonica so I will send it over when I get it. I don't think
> we are supporting @2x graphic sizes. Patryk can you confirm?

Hi Amy,

I think we do need to support @2x.
For example, you may see we have some graphics @2x in browser app, 
https://github.com/mozilla-b2g/gaia/tree/master/apps/browser/style/images

Could you please help on that?
Thanks.
(In reply to Amy from comment #10)
> Created attachment 774613 [details]
> PDF Assets
> 
> Hi Rudy, 
> 
> Attached are the right/left div lines.

Hi Amy,

For the div lines, the 1x resolution is 2 x 34, so for @1.5x, I think we need "3 x 51". (right now, it's 2 x 51)

Could you help provide those assets again?
Thank you.
(In reply to Rudy Lu [:rudyl] from comment #13)
> (In reply to Amy from comment #10)
> > Created attachment 774613 [details]
> > PDF Assets
> > 


Hi Rudy, 

The hairline looks too thick at 3px so we are keeping it at 2px. As for the @2x graphics this is Patryk's response:

"As far as the @2x.
This is only supported on the dev. preview phone, the Geeksphone Peak. I haven't heard that we are supporting these phones past release. We should focus on the @1.5x work anyway, as there is a hard deadline Aug 10th."
Attachment #774613 - Flags: feedback?(padamczyk)
The 1.5x assets have been piled and I just sent a pull request to pdf.js
https://github.com/mozilla/pdf.js/pull/3479
(In reply to Rudy Lu [:rudyl] from comment #15)
> The 1.5x assets have been piled and I just sent a pull request to pdf.js
> https://github.com/mozilla/pdf.js/pull/3479

It had been merged!
Flags: needinfo?(rlu)
Created attachment 777663 [details]
Patch to update pdf.js

This is the pull request to update pdf.js to 0.8.0352.
Thanks.
Attachment #777663 - Flags: review?(bdahl)
Flags: needinfo?(rlu)
Whiteboard: [GAIA_HD_ASSETS]
Depends on: 895312
No longer depends on: 895312
Hi Brendan, 

I found Bug 895312 should be a gecko issue and has been resolved.
Could you pleas help review attachment 777663 [details] or should I find someone else to help?

Thank you.
Flags: needinfo?(bdahl)
Comment on attachment 777663 [details]
Patch to update pdf.js

Hi Vivien,

Any chance you can help review this change to update pdf.js in Gaia?
Thank you.
Attachment #777663 - Flags: review?(21)
Comment on attachment 777663 [details]
Patch to update pdf.js

Sounds like the changes have landed on pdf.js upstream. Let's update our version.

Thanks folks for working together :)
Attachment #777663 - Flags: review?(bdahl)
Attachment #777663 - Flags: review?(21)
Attachment #777663 - Flags: review+
Landed to Gaia master,
4e16dc5dc63d706a3f40496a71f30e4bb4ae103e

v1.1.0hd branch, 
f4ad65e688dba4555ade50027687214c7e686b0b

--
Vivien,

Thanks for the review.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-b2g-v1.1hd: --- → fixed
Flags: needinfo?(bdahl)
Resolution: --- → FIXED

Updated

5 years ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.