Closed Bug 1217357 Opened 4 years ago Closed 4 years ago

Update broken call to getContentWindow() from larch merge

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(firefox44 fixed)

RESOLVED FIXED
FxOS-S10 (30Oct)
Tracking Status
firefox44 --- fixed

People

(Reporter: timdream, Assigned: timdream)

References

Details

Attachments

(1 file)

https://hg.mozilla.org/mozilla-central/annotate/f029ccdee154bdc2b49a1e0fcd5b0fa0397aa97b/b2g/chrome/content/shell.js#l108 

bug 1160923 removed getContentWindow() but bug 1204965 merged some code containing that. It should be updated to either

shell.contentBrowser.contentWindow

or 

SystemAppProxy._frame.contentWindow

Apparently the branch does not contain any test to realize this feature is broken? I can help with a drive-by patch tomorrow or someone can beat me to it.

PS I really don't like these untested global functions being checked into shell.js. Not sure who can make the call here and if the said person really cares.
Oops, accidentally submitted the form, was going to say:

Thanks for filing, sorry about merging this in a broken state - it should definitely have tests. I can't get to this immediately but may be able to take it down the road if you don't get to it.
Attached patch bug1217357.patchSplinter Review
Flags: needinfo?(kevingrandon)
Attachment #8677848 - Flags: review?(kevingrandon)
Comment on attachment 8677848 [details] [diff] [review]
bug1217357.patch

Review of attachment 8677848 [details] [diff] [review]:
-----------------------------------------------------------------

Not too familiar with this stuff, but this particular getter seems to be all over shell.js so it makes sense to me. Thank you very much for fixing it.
Attachment #8677848 - Flags: review?(kevingrandon) → review+
https://hg.mozilla.org/mozilla-central/rev/3b5812d1023c
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S10 (30Oct)
You need to log in before you can comment on or make changes to this bug.