Closed Bug 1141123 Opened 6 years ago Closed 3 years ago

Remove inline css in fl app


(Firefox OS Graveyard :: Gaia, defect)

Not set


(Not tracked)



(Reporter: miot.myriam, Unassigned)




(3 files)

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
Submitting a fix for this bug
Attachment #8574738 - Flags: review?(dflanagan)
Blocks: 968907
Comment on attachment 8574738 [details] [review]


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 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 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
Attachment #8574738 - Flags: review?(dflanagan) → review-
Attachment #8574738 - Flags: review- → review?(dflanagan)
Attached image 2015-03-11-15-29-59.png

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.

Flags: needinfo?(dflanagan)
Attachment #8576005 - Flags: review?(dflanagan)
Comment on attachment 8576005 [details]

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.
Flags: needinfo?(dflanagan)
Attachment #8576005 - Flags: review?(dflanagan) → review+
Comment on attachment 8574738 [details] [review]

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.
Attachment #8574738 - Flags: review?(dflanagan) → review-
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.
Firefox OS is not being worked on
Closed: 3 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.