Closed Bug 1144450 Opened 5 years ago Closed 5 years ago

infinite recursion when inspecting document.fonts.status from loadingdone event listener

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: heycam, Assigned: heycam)

References

Details

Attachments

(5 files)

In the extended test in bug 1086207, we get:

  JavaScript error: file:///tmp/test.html, line 53: too much recursion

This happens due to accessing document.fonts.status from within the loadeddone event listener.
I think the problem is that in FontFaceSet::UpdateRules, we do not have the same "check to see that the FontFaces we're adding already existed in the same order" for non-rule FontFaces that we do for rule FontFaces.  This means every time we call UpdateRules when there is a non-rule FontFace present, we end up thinking that there was a change the font face set, which means calling CheckLoadingStarted.

CheckLoadingStarted also has a problem (I think) that it replaces mReady with a new promise (and sets mReadyIsResolved to false, which is the cause of the infinite recursion) even when HasLoadingFontFaces() return false.  We should only be replacing mReady at the same time as setting mStatus to Loading and dispatching the loading event.
In this bug I will fix the latter problem.  Fixing the former will then become an optimization to save a bit of work.
Assignee: nobody → cam
Status: NEW → ASSIGNED
Attachment #8579816 - Flags: review?(jdaggett)
Attachment #8579816 - Flags: review?(jdaggett) → review+
Attachment #8579817 - Flags: review?(jdaggett) → review+
Attachment #8579818 - Flags: review?(jdaggett) → review+
Attachment #8579819 - Flags: review?(jdaggett) → review+
Attachment #8579820 - Flags: review?(jdaggett) → review+
You need to log in before you can comment on or make changes to this bug.