1650686 - Drop the fullscreen state check in ScrollFrameHelper::UpdateMinimumScaleSize along with dropping layout.viewport_contains_no_contents_area check in the same function

Status: RESOLVED FIXED

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

Description

[Hiroyuki Ikezoe (:hiro)]

here and here.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1650686 - Drop fullscreen state check in ScrollFrameHelper::UpdateMinimumScaleSize. r?botond

I've confirmed that the issue originally reported in bug 1520455 doesn't happen
on an archived version of the site in question [1] without this check.

[1] https://web.archive.org/web/20190118042101/https://www.mozilla.org/en-US/firefox/

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1650686 - Drop layout.viewport_contains_no_contents_area. r?botond

I suppose it's been well tested on Fenix.

Depends on D82304

Comment 3

[Hiroyuki Ikezoe (:hiro)]

test_group_fullscreen.html seems to be a test case relies on the fullscreen state check. Though it looks like I wrote the test, I can recall anything right now, I will look into detail tomorrow. I am going to land another part now.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ee17c86a69b4
Drop layout.viewport_contains_no_contents_area. r=botond

Comment 5

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/ee17c86a69b4

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Leaving NI to me. I couldn't have enough time on investigation the failure reason of test_group_fullscreen.html (in comment 3). It looks like nsIDOMWindowUtils.scrollToVisual shouldn't work in fullscreen state, but I am not enough confident yet.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Do you know if the remaining patch here will fix bug 1659761? (I'm having trouble reproducing that bug so I'm not sure I'll be able to test it myself)

Comment 8

[Hiroyuki Ikezoe (:hiro)]

I actually droped the check to see whether it fixes/affects bug 1659761, it's not at all.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

Can we close this bug and file a follow-up for whatever else needs to be done? It gets confusing when patches landed a long time ago but the bug is still marked open. Also the stale needinfo on this bug is causing it stick in the APZ triage queue.

Comment 10

[Hiroyuki Ikezoe (:hiro)]

Yep, sure. Done. Bug 1676055 is the new one.

Comment 11

[Kartikaya Gupta (email:[e-mail])]

Thanks!