1141123 - Remove inline css in fl app

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia, defect)

Description

[Myriam]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:36.0) Gecko/20100101 Firefox/36.0
Build ID: 20150224133805

Steps to reproduce:

We need to remove inline css - see Bug 968907

 <p style="text-align: center;">  in apps/fl/download.html at l.32

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] myriamiot:bug-1141123 > mozilla-b2g:master

Comment 2

[Myriam]

Attachment: https://github.com/mozilla-b2g/gaia/pull/28725

Submitting a fix for this bug

Comment 3

[David Flanagan [:djf]]

Comment on attachment 8574738 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/28725

Myriam,

Welcome! Thanks for contributing to FirefoxOS.

I've given an r- to your proposed patch, which means that I don't think it is ready yet to land.  I hope you'll keep working on it, though, because I think it should be pretty easy to get right.

Note that in download.html there is another <progress> element a few lines down from the one you're fixing. It gets centered in download.css because its size and margins are set.  I'd prefer a fix that treated this first progress element like the second one. I'd guess you could remove the <p> tag, and modify the download.css stylesheet so that it applies the same styles to both #download-progress and #direct-download-progress.

This bug is also tricky because it is not at all obvious how to launch the FL app to test your fix. Do you have a FirefoxOS device you can test on? I have no idea if it works in desktop versions or not.

The FL app handles downloads of "Forward Locked" content. This is a weak form of DRM, used by some phone operators to protect ringtones and wallpapers that they sell.

To invoke and test the app, you can visit djf.net/drmtests/ to see a bunch of links.

If you click on the ".dd" links you'll get the "download-progress" progress bar.  If you click on one of the .dm links, you'll get the direct-download-progress bar. (Surprisingly, ".dd" does not stand for "direct download", it is hte .dm files that are directly downloaded).

You'll need to select one of the big files to have the progress bar displayed for very long, so maybe the FL_Binary_PNG_424K.dm file would be a good one to test with.

If you can manage to get screenshots of the centered progress bar both before and after your patch, that would be really helpful to prove that your patch works as expected. (You can attach images to the bug just like you'd attach the patch.)

Thanks again for contributing. Please let me know if you have questions. The best way to do that is to check the "need more information from" box here in bugzilla and enter :djf in the text field. (Or you can try emailing me directly [e-mail])

Comment 4

[Myriam]

Attachment: 2015-03-11-15-29-59.png

Hi,

Thank you for your advice.
I've just changed the download.html and the download.css files
Here is a screenshot of the result.
I hope it is fine.
Thanks

Myriam

Comment 5

[David Flanagan [:djf]]

Comment on attachment 8576005 [details]
2015-03-11-15-29-59.png

The screenshot looks good.  Generally you don't need to request technical review for screenshots; I'll look at it as part of the patch review. Sometimes, when a UX designer is involved, we request a ux review on attachments like this.

Comment 6

[David Flanagan [:djf]]

Comment on attachment 8574738 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/28725

Thanks for continuing to work on this. I like the approach you took in this revised patch, but there are two syntax errors. See my comments on github. They should be very easy to fix, and I'd expect that I'll give an r+ on the next review.

Comment 7

[Myriam]

Hello David,
Sorry for the delay. I did the 2 small corrections. I hope it's ok. Thank you for your comments and especially for taking the time to explain to me. Thank you very much.

Comment 8

[BMO Automation]

Firefox OS is not being worked on