The default bug view has changed. See this FAQ.

matchMedia() listeners lost

RESOLVED FIXED in mozilla13

Status

()

Core
DOM: CSS Object Model
P2
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Nicholas C. Zakas, Assigned: dbaron)

Tracking

Trunk
mozilla13
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
User Agent: Mozilla/5.0 (Windows NT 5.1) AppleWebKit/535.11 (KHTML, like Gecko) Chrome/17.0.963.26 Safari/535.11

Steps to reproduce:

window.matchMedia("screen and (max-width:600px)").addListener(function(media){
    console.log("HERE: " + window.innerWidth);
});

Then resized my browser several times.


Actual results:

The event handler fired three times and then stopped. In some cases, it fired zero times.


Expected results:

It should have continued to fire repeatedly. The only way to make this happen was to cache the MediaQueryList object, such as:

var mql = window.matchMedia("screen and (max-width:600px)");
mql.addListener(function(media){
    console.log("HERE: " + window.innerWidth);
});
Yeah, we need to hold an ownin reference from the window/document/presshell to the returned object as long as it has listeners registered. Also, this should have used events, oh well.
Assignee: nobody → dbaron
Component: Untriaged → Layout
Product: Firefox → Core
QA Contact: untriaged → layout
Version: 9 Branch → Trunk
(Assignee)

Updated

5 years ago
Assignee: dbaron → nobody
Component: Layout → DOM: CSS Object Model
QA Contact: layout → style-system
(Assignee)

Updated

5 years ago
Assignee: nobody → dbaron
OS: Windows XP → All
Hardware: x86 → All
(Assignee)

Updated

5 years ago
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Assignee)

Comment 2

5 years ago
Created attachment 601686 [details] [diff] [review]
patch
Attachment #601686 - Flags: review?(bzbarsky)
Comment on attachment 601686 [details] [diff] [review]
patch

mListeners is a fallible array, right?  So we could addref but fail to append in AddListener, then end up leaking on the next AddListener call, as far as I can tell.
Attachment #601686 - Flags: review?(bzbarsky) → review-
(Assignee)

Comment 4

5 years ago
Oh, for some reason I read that as InfallibleTArray.  I'd sort of thought that's what I'd written, too.  But so it is.

The most straightforward fix would seem to be (in AddListener):

   if (!mListeners.Contains(aListener)) {
     mListeners.AppendElement(aListener);
+    if (!HasListeners()) {
+      // Append failed; undo the AddRef above.
+      NS_RELEASE_THIS();
+    }
   }
Yes.  r=me with that change.
(Assignee)

Comment 6

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a388b0077d5
Priority: -- → P2
Target Milestone: --- → mozilla13
https://hg.mozilla.org/mozilla-central/rev/2a388b0077d5
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.