mozilla :: #apz

14 Feb 2017
21:46botondtnikkel: so, yeah, checking mStopped fixes that scroll pos restoration bug!
22:00tnikkelbotond: cool. does it regress anything else? or violate the intent of any of the previous 10 or so patches to that code?
22:04botondtnikkel: i'm doing a try push to see
22:04botondtnikkel: as for violating the intent... do you know under what conditions Stop is called?
22:45tnikkelbotond: i guess anytime anyone wants to stop the document load before its complete? i'm not 100% clear on it
22:46botondtnikkel: so, for example, if I click in "x" button next to the address bar (where usually the reload button is?)
22:46botondclick the*
22:46tnikkelbotond: yeah, thats one way
22:47botondtnikkel: so, let's consider the implications of that
22:47botondtnikkel: i'm on a page, scroll down to y=500
22:47botondtnikkel: press reload. it takes long, and a get bored. i press "x" before any content is visible
22:48botondtnikkel: then i decide to try again, press "reload". do i want the scroll position to go to y=500?
22:48botondtnikkel: i'm inclined to say "yes"
22:50tnikkelbotond: yeah
22:52botondtnikkel: ok. i believe the patch should produce that effect (hard to test because it's hard to find a page that takes so long to load i actually get a chance to press "x")
22:52tnikkelbotond: i was more thinking about the patch from the bug that cause this regression, and the series of bugs that preceded it. it seems like there is a delicate balance in this code
22:56botondtnikkel: heh, i just saw what you wrote in "I also noticed that PresShell::LoadComplete isn't quite what we want to be tracking as it only gets called on success. We also want to stop restoring on error or if the document loading was stopped. nsDocumentViewer::LoadComplete sets mLoaded, which is what I think we should track as for "loading/not loading"."
22:56firebotBug 1269539 FIXED, Scroll position lost when reload page
23:11botondtnikkel: do you recall why, at the time, you believed we wanting to stop restoring if the document loading was stopped?
23:32tnikkelbotond: that meant that there was no more content we were expecting to add to the page length
23:32tnikkelso if we kept trying to restore if the page changed size later we would wrongly restore
23:33botondtnikkel: well, no more content we were expecting to add until the next complete (meaning, not stopped) load
23:34tnikkelbotond: hmm, but the page may not reload
23:35botondtnikkel: right, it may not necessarily reload until further user action. that's the case in the scenario i describe above (user presses stop, then reload). but, at least in that case, we agree that restoring after the subsequent reload is desirable?
23:38tnikkelbotond: after the restore yes. but the "stopped" page would still be susceptible to the same bugs which caused us to clear after load is complete
23:50botondtnikkel: do you mean like bug 1266833?
23:50firebot FIXED, [e10s] Setting overflow:hidden; and height: 100%; on html/body element causes issues if scrolled dow
15 Feb 2017
No messages
Last message: 222 days and 14 hours ago