Bug 1648839 - Evaluate changes in all media queries, then fire change events. r=jwatt

This ensures that you can't observe an inconsistent state while we go
through the list.

It should also be marginally better as we don't build an array with all
the media queries unconditionally.

Differential Revision: https://phabricator.services.mozilla.com/D82260
This commit is contained in:
Emilio Cobos Álvarez 2020-07-10 01:21:53 +00:00
Родитель 9ec63950e7
Коммит 8be683efd5
4 изменённых файлов: 52 добавлений и 15 удалений

Просмотреть файл

@ -1561,10 +1561,15 @@ void nsPresContext::FlushPendingMediaFeatureValuesChanged() {
mDocument->NotifyMediaFeatureValuesChanged();
// https://drafts.csswg.org/cssom-view/#evaluate-media-queries-and-report-changes
//
// Media query list listeners should be notified from a queued task
// (in HTML5 terms), although we also want to notify them on certain
// flushes. (We're already running off an event.)
//
// TODO: This should be better integrated into the "update the rendering"
// steps: https://html.spec.whatwg.org/#update-the-rendering
//
// Note that we do this after the new style from media queries in
// style sheets has been computed.
@ -1574,22 +1579,20 @@ void nsPresContext::FlushPendingMediaFeatureValuesChanged() {
// We build a list of all the notifications we're going to send
// before we send any of them.
// Copy pointers to all the lists into a new array, in case one of our
// notifications modifies the list.
nsTArray<RefPtr<mozilla::dom::MediaQueryList>> localMediaQueryLists;
nsTArray<RefPtr<mozilla::dom::MediaQueryList>> listsToNotify;
for (MediaQueryList* mql = mDocument->MediaQueryLists().getFirst(); mql;
mql = static_cast<LinkedListElement<MediaQueryList>*>(mql)->getNext()) {
localMediaQueryLists.AppendElement(mql);
if (mql->MediaFeatureValuesChanged()) {
listsToNotify.AppendElement(mql);
}
}
nsContentUtils::AddScriptRunner(NS_NewRunnableFunction(
"nsPresContext::FlushPendingMediaFeatureValuesChanged",
[list = std::move(localMediaQueryLists)] {
// Now iterate our local array of the lists.
[list = std::move(listsToNotify)] {
for (const auto& mql : list) {
nsAutoMicroTask mt;
mql->MaybeNotify();
mql->FireChangeEvent();
}
}));
}

Просмотреть файл

@ -131,21 +131,20 @@ JSObject* MediaQueryList::WrapObject(JSContext* aCx,
return MediaQueryList_Binding::Wrap(aCx, this, aGivenProto);
}
void MediaQueryList::MaybeNotify() {
bool MediaQueryList::MediaFeatureValuesChanged() {
mMatchesValid = false;
if (!HasListeners()) {
return;
return false; // No need to recompute or notify if we have no listeners.
}
bool oldMatches = mMatches;
RecomputeMatches();
// No need to notify the change.
if (mMatches == oldMatches) {
return;
}
return mMatches != oldMatches;
}
void MediaQueryList::FireChangeEvent() {
MediaQueryListEventInit init;
init.mBubbles = false;
init.mCancelable = false;

Просмотреть файл

@ -41,7 +41,10 @@ class MediaQueryList final : public DOMEventTargetHelper,
nsISupports* GetParentObject() const;
void MaybeNotify();
// Returns whether we need to notify of the change event using
// FireChangeEvent().
[[nodiscard]] bool MediaFeatureValuesChanged();
void FireChangeEvent();
JSObject* WrapObject(JSContext* aCx,
JS::Handle<JSObject*> aGivenProto) override;

Просмотреть файл

@ -0,0 +1,32 @@
<!doctype html>
<title>MediaQueryList.changed is correct for all lists in the document even during a change event handler</title>
<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
<link rel="author" href="https://mozilla.org" title="Mozilla">
<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1648839">
<link rel="help" href="https://drafts.csswg.org/cssom-view/#evaluate-media-queries-and-report-changes">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="resources/matchMedia.js"></script>
<body>
<script>
promise_test(async t => {
// Create two identical media queries.
let mql = await createMQL(t);
let mql2 = getWindow(mql).matchMedia(mql.media);
let changeEvents = 0;
let check = t.step_func(function() {
changeEvents++;
assert_equals(mql.matches, mql2.matches, "Value of .matches should match"); // No pun intended
});
mql.addListener(check);
mql2.addListener(check);
triggerMQLEvent(mql);
await waitForChangesReported();
assert_equals(changeEvents, 2, "Should've fired the change event in both MediaQueryLists");
});
</script>