Closed Bug 1512838 Opened 2 years ago Closed 1 year ago

Resolution doesn't get applied after one touch pinch

Categories

(Core :: Panning and Zooming, defect, P3)

65 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- wontfix
firefox67 --- fixed

People

(Reporter: kats, Assigned: kats)

References

Details

Attachments

(3 files)

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.
Duplicate of this bug: 1524917
Blocks: 1524917

Bug 1111333 - the original for reference

Duplicate of this bug: 1528073
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
No longer blocks: 1524917
Duplicate of this bug: 1524917
No longer depends on: wr-android-mvp
Regressions: 1536626
You need to log in before you can comment on or make changes to this bug.