Resolution doesn't get applied after one touch pinch

RESOLVED FIXED in Firefox 67

Status

()

defect
P3
normal
RESOLVED FIXED
7 months ago
3 months ago

People

(Reporter: kats, Assigned: kats)

Tracking

(Blocks 1 bug)

65 Branch
mozilla67
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox64 wontfix, firefox65 wontfix, firefox66 wontfix, firefox67 fixed)

Details

Attachments

(3 attachments)

Fennec nightly, do a one touch pinch on a bugzilla page. It remains blurry. Looks like the main thread doesn't take the new resolution, because pinching afterwards resets the zoom back to what it was before the one touch pinch.
Not a recent regression: issue goes at least as far back as Firefox 62.
And Firefox 55, which is around the time one-touch pinch was introduced.
Priority: -- → P3
Though it's not the case that one-touch pinch is just completely broken, as e.g. on planet.mozilla.org it works fine (increasing resolution and all).
Maybe we're trying to apply the zoom on the scrollable div instead of the RCD.

Bug 1111333 - the original for reference

Assignee: nobody → kats

The patch for this is simple enough. Time to write some tests! rolls up sleeves

Sometimes we can get empty transactions after a scrollframe is
layerized. In such cases the isLayerized check would incorrectly detect
the scrollframe as not being layerized because it would only look at the
data for the empty transaction.

Depends on D23494

The helper_basic_onetouchpinch.html is basically a copy of
helper_basic_zoom.html with a few things changed (most importantly, the
touch event sequence).

Depends on D23495

Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/754f833aaa41
Redirect one-touch-pinch scale gestures to the root content APZC. r=botond
https://hg.mozilla.org/integration/autoland/rev/6bd80d61cee8
Skip over empty transaction buckets in isLayerized. r=botond
https://hg.mozilla.org/integration/autoland/rev/589f41b2e253
Add mochitests to exercise the one-touch-pinch code. r=botond

Huh. That seems to be a legitimate bug in nsDOMWindowUtils::SetDisplayPortForElement, introduced in this commit:

  • We query an element property here and keep a raw pointer to it.
  • We overwrite the property here, which frees the old property object.
  • We subsequently dereference the pointer pointing to the old data here (this is the part that was added in the change mentioned above).

(In reply to Botond Ballo [:botond] from comment #16)

Huh. That seems to be a legitimate bug in
nsDOMWindowUtils::SetDisplayPortForElement, introduced in [this
commit](https://hg.mozilla.org/mozilla-central/rev/
484e07a716f1aac33b4fd9ee246c7eab8b504523):

-> bug 1535862.

Thanks for the quick diagnosis and fix! I'll reland these patches on Monday.

Depends on: 1535862
Flags: needinfo?(kats)
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/aededeb1f164
Redirect one-touch-pinch scale gestures to the root content APZC. r=botond
https://hg.mozilla.org/integration/autoland/rev/729056f6dc85
Skip over empty transaction buckets in isLayerized. r=botond
https://hg.mozilla.org/integration/autoland/rev/cb58de9731cb
Add mochitests to exercise the one-touch-pinch code. r=botond
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
No longer blocks: 1524917
Duplicate of this bug: 1524917
You need to log in before you can comment on or make changes to this bug.