"prefStrBundle.getFormattedString is not a function" exceptions in in-content/advanced.js

RESOLVED FIXED in Firefox 48

Status

()

Firefox
Preferences
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: bz, Assigned: jaws)

Tracking

unspecified
Firefox 48
Points:
---

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

I was looking at a log for a M(BC5) run on Linux64 (e.g. <http://archive.mozilla.org/pub/firefox/try-builds/bzbarsky@mozilla.com-4b1f80c98bed138964e10728195be517e8441800/try-linux64-debug/try_ubuntu64_vm-debug_test-mochitest-browser-chrome-5-bm124-tests1-linux64-build173.txt.gz> for a nice passing run) and saw four exceptions like this in the log:

20:27:41     INFO -  JavaScript error: chrome://browser/content/preferences/in-content/advanced.js, line 313: TypeError: prefStrBundle.getFormattedString is not a function

during the rowser/components/preferences/in-content/tests/browser_bug795764_cachedisabled.js test.

This seems pretty odd to me, and I haven't managed to reproduce it locally yet by actually going to the advanced pref pane...

Comment 1

2 years ago
(In reply to Boris Zbarsky [:bz] from comment #0)
> I was looking at a log for a M(BC5) run on Linux64 (e.g.
> <http://archive.mozilla.org/pub/firefox/try-builds/bzbarsky@mozilla.com-
> 4b1f80c98bed138964e10728195be517e8441800/try-linux64-debug/try_ubuntu64_vm-
> debug_test-mochitest-browser-chrome-5-bm124-tests1-linux64-build173.txt.gz>
> for a nice passing run) and saw four exceptions like this in the log:
> 
> 20:27:41     INFO -  JavaScript error:
> chrome://browser/content/preferences/in-content/advanced.js, line 313:
> TypeError: prefStrBundle.getFormattedString is not a function
> 
> during the
> rowser/components/preferences/in-content/tests/
> browser_bug795764_cachedisabled.js test.
> 
> This seems pretty odd to me, and I haven't managed to reproduce it locally
> yet by actually going to the advanced pref pane...

The test in question opens the preferences, then the advanced pane, then does some sync checks on which elements are visible, and then closes the tab.

The advanced prefs do some async fetches of storage/cache information and then display the results. If those fetches come in when the tab has already closed, the document and its nodes might be alive (so you can getElementById the string bundle and get an element back) but the XBL binding will be dead, and so getFormattedString is gone.

There's at least a few options here....
1) try to see if we can remove the observers (not sure we can in all of the cases in advanced.js as they're going through the storage and cache service) when the page unloads
2) null-check getFormattedString and bail out of the observers;
3) do nothing because this only happens in automated tests and has no real ill effects if you managed to reproduce it in normal browser usage

I don't really feel strongly either way. Jared?
Flags: needinfo?(jaws)
Option #3 is my choice here.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Flags: needinfo?(jaws)
Resolution: --- → WONTFIX
So to be clear, this is blocking me landing some changes to xpconnect to improve error reporting.  In particular, the changes cause the exception in this case to actually be associated with the window the code is in, instead of the junk scope as now.  That keeps the window object alive for a little bit after the test (I'm still hunting down exactly what does that).  And then the leaked window detection in browser-test.js fails the test suite.
Sorry, I didn't realize this was blocking you. We can do option #2 then as it is simple and should prevent the error, as well we should add a comment describing what is happening here.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Awesome, thanks.  I'm still trying to figure out why the window stays alive longer, by the way; so far I haven't found something that would actually cause that...

Comment 6

2 years ago
(In reply to Boris Zbarsky [:bz] from comment #5)
> Awesome, thanks.  I'm still trying to figure out why the window stays alive
> longer, by the way; so far I haven't found something that would actually
> cause that...

Any chance that's actually related to these observers (could just comment the whole bunch of code here out to see) ?
If I just try/catch around the calls to getFormattedString in these observers, my oranges go away.  So it's definitely related to the exceptions thrown by these observers...  I'm not sure that's what you're asking though?
Created attachment 8727531 [details]
MozReview Request: Bug 1253891 - "prefStrBundle.getFormattedString is not a function" exceptions in in-content/advanced.js. r?gijs

Guards were added to both async calls as it is likely that either of them could fire after the tab has been closed.

Review commit: https://reviewboard.mozilla.org/r/38499/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38499/
Attachment #8727531 - Flags: review?(gijskruitbosch+bugs)

Comment 9

2 years ago
Comment on attachment 8727531 [details]
MozReview Request: Bug 1253891 - "prefStrBundle.getFormattedString is not a function" exceptions in in-content/advanced.js. r?gijs

https://reviewboard.mozilla.org/r/38499/#review35095

r=me but the check for the element itself is not currently strictly necessary AIUI.
Attachment #8727531 - Flags: review?(gijskruitbosch+bugs) → review+
Thanks. I added the check for the element because it seemed like extra added protection, but I'll remove it.
Assignee: nobody → jaws
Status: REOPENED → ASSIGNED
Created attachment 8727542 [details] [diff] [review]
Patch for check-in
Attachment #8727531 - Attachment is obsolete: true
Attachment #8727542 - Flags: review+
Keywords: checkin-needed

Comment 13

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c4f5d75e11f2
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
You need to log in before you can comment on or make changes to this bug.