701288 - font loader needs to handle redirect requests better

Status: RESOLVED WORKSFORME

(Core :: CSS Parsing and Computation, defect, P3)

Description

[John Daggett (:jtd)]

This is a follow-up from bug 701262.  The code in nsFontDownloader keeps track of the initial channel used to start a request.  However, the stream loader will switch to another channel if a redirect occurs.  Since the timer code that handles whether to redraw with fallback fonts or not is using the initial channel, it won't give proper behavior when redirects occur.

Code changes needed:

- the stream loader code only updates the channel when requests have stopped, it really should be updating that channel when a channel change occurs

- the timer code in nsFontDownloader should be using the stream loader to fetch the current channel when checking the progress of a font download

Comment 1

[Boris Zbarsky [:bzbarsky]]

The other issue is that right now nsFontFaceLoader::Cancel is broken if mChannel gets redirected.

We could make the stream loader a redirect observer, I guess.  We could also make the nsFontFaceLoader a redirect observer on the channel...

Comment 2

[John Daggett (:jtd)]

But if the stream loader updates the current channel, then the cancel code just needs to access the channel via the stream loader, no?

Comment 3

[Boris Zbarsky [:bzbarsky]]

Yes, once this bug is fixed Cancel will work.  Just pointing out that it's not just the timer thing that's broken right now.

Comment 4

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Please make sure that redirects for fonts from https:// -> http:// have the same behavior regarding security indicators/mixed content as a direct http:// load would have. In particular, the font should not load in such a situation if the document is HTTPS, just like it shouldn't load if the original font URL was http://. (Do we have a test for that.)

Comment 5

[John Daggett (:jtd)]

Brian, I will confirm that it does but if it does not the bug would be in nsCORSListenerProxy code.  The font loading code doesn't explicitly check for same origin.  See the code in layout/style/nsFontFaceLoader.cpp.

Comment 6

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information

Comment 7

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

In the process of migrating remaining bugs to the new severity system, the severity for this bug cannot be automatically determined. Please retriage this bug using the new severity system.

Comment 8

[Daniel Holbert [:dholbert]]

[ran across this while triaging old bugs that don't have a severity field set]

I suspect there's nothing left to be done here. ni=jfkthame to take a quick look & confirm if he has cycles.

In particular:

• comment 0 references nsFontDownloader, which no longer exists anywhere in the tree.
• comment 1 mentions some redirect issues, which I suspect may have been fixed via the patch that ended up landing in bug 701262 (?) but I'm not entirely sure. I suspect we would've found out by now if comment 1 was an issue, though.