59877 2011-05-01 10:05:22 -0700 [chromium]: Scroll position not restored on navigation back for in-page links 2022-02-27 23:35:49 -0800 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED http://www.w3.org/TR/2011/WD-css3-text-20110412/ InRadar P2 Normal --- 1 thakis japhet abarth ap bdakin eric mihaip progame+wk tonikitoo webkit.review.bot oldest_to_newest 395935 0 thakis 2011-05-01 10:05:22 -0700 1. Go to a page with in-page links, e.g. http://www.w3.org/TR/2011/WD-css3-text-20110412/ 2. Scroll down to table of contens 3. Click any topic 4. Click "Back" Expected: Scroll position from after step 2 is restored Actual: Back button jumps to top of page. 396250 1 ap 2011-05-01 23:18:45 -0700 <rdar://problem/9366060> 396650 2 mihaip 2011-05-02 14:13:35 -0700 I believe this has been fixed already by http://trac.webkit.org/changeset/84604. I'm not able to reproduce this with the most recent WebKit nightly (at r85429), but it does happen with one at r84547. *** This bug has been marked as a duplicate of bug 57898 *** 396653 3 thakis 2011-05-02 14:15:43 -0700 Mihai: The problem repros for me in the chrome canary, which uses webkit r85369. 396659 4 thakis 2011-05-02 14:20:25 -0700 Turns out this doesn't repro in a webkit nightly. Sorry. 397719 5 progame+wk 2011-05-03 23:50:34 -0700 reproducible using Google Chrome 13.0.754.0 (Official Build 83837) canary WebKit 534.33 (trunk@85565) with the steps: 1. visit http://ruby.railstutorial.org/chapters/rails-flavored-ruby#sec:strings 2. click on another in page link in that page (to a different location, for example "4.2.1 Comments" which is just above the original link position) 3. clicking back will bring you back to "4.2.2 Strings" instead of the location you viewed when clicking on that other link 398221 6 92312 japhet 2011-05-04 13:47:25 -0700 Created attachment 92312 patch 398246 7 92312 mihaip 2011-05-04 14:13:08 -0700 Comment on attachment 92312 patch View in context: https://bugs.webkit.org/attachment.cgi?id=92312&action=review > Source/WebKit/chromium/src/WebFrameImpl.cpp:1045 > + if (!m_inSameDocumentHistoryLoad && (m_frame->loader()->loadType() == FrameLoadTypeStandard Any ideas as to why we save the scroll state in a getter in the first place? As far as I can tell, that's been there since this code was upstreamed, but it still seems odd. I would be curious what breaks if we remove this altogether, but that can be done separately, since I believe this is blocking a Chromium release. Maybe just a FIXME? 398250 8 japhet 2011-05-04 14:18:07 -0700 (In reply to comment #7) > (From update of attachment 92312 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=92312&action=review > > > Source/WebKit/chromium/src/WebFrameImpl.cpp:1045 > > + if (!m_inSameDocumentHistoryLoad && (m_frame->loader()->loadType() == FrameLoadTypeStandard > > Any ideas as to why we save the scroll state in a getter in the first place? As far as I can tell, that's been there since this code was upstreamed, but it still seems odd. > > I would be curious what breaks if we remove this altogether, but that can be done separately, since I believe this is blocking a Chromium release. Maybe just a FIXME? I don't know the full story, but I assume the comment is correct in that we can clobber HistoryItems otherwise. I've got a rare null-deref crash outstanding in this function, so I'll look into it more carefully in the near-ish future. I'll add a FIXME before landing. 398257 9 japhet 2011-05-04 14:32:53 -0700 Landed: http://trac.webkit.org/changeset/85787 398303 10 webkit.review.bot 2011-05-04 15:21:50 -0700 http://trac.webkit.org/changeset/85787 might have broken Leopard Intel Debug (Tests) The following tests are not passing: svg/text/text-block-child-crash.xhtml 92312 2011-05-04 13:47:25 -0700 2022-02-27 23:35:49 -0800 patch hi.txt text/plain 2937 japhet SW5kZXg6IFNvdXJjZS9XZWJLaXQvY2hyb21pdW0vc3JjL1dlYkZyYW1lSW1wbC5jcHAKPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gU291cmNlL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViRnJhbWVJbXBsLmNwcAkocmV2 aXNpb24gODU3NjYpCisrKyBTb3VyY2UvV2ViS2l0L2Nocm9taXVtL3NyYy9XZWJGcmFtZUltcGwu Y3BwCSh3b3JraW5nIGNvcHkpCkBAIC05NDUsOCArOTQ1LDEwIEBACiAgICAgICAgIG1fZnJhbWUt PnBhZ2UoKS0+YmFja0ZvcndhcmQoKS0+c2V0Q3VycmVudEl0ZW0oY3VycmVudEl0ZW0uZ2V0KCkp OwogICAgIH0KIAorICAgIG1faW5TYW1lRG9jdW1lbnRIaXN0b3J5TG9hZCA9IGN1cnJlbnRJdGVt LT5zaG91bGREb1NhbWVEb2N1bWVudE5hdmlnYXRpb25UbyhoaXN0b3J5SXRlbS5nZXQoKSk7CiAg ICAgbV9mcmFtZS0+cGFnZSgpLT5nb1RvSXRlbShoaXN0b3J5SXRlbS5nZXQoKSwKICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgIEZyYW1lTG9hZFR5cGVJbmRleGVkQmFja0ZvcndhcmQpOwor ICAgIG1faW5TYW1lRG9jdW1lbnRIaXN0b3J5TG9hZCA9IGZhbHNlOwogfQogCiB2b2lkIFdlYkZy YW1lSW1wbDo6bG9hZERhdGEoY29uc3QgV2ViRGF0YSYgZGF0YSwKQEAgLTEwNDAsOCArMTA0Miw4 IEBACiAgICAgLy8gSWYgd2UgYXJlIHN0aWxsIGxvYWRpbmcsIHRoZW4gd2UgZG9uJ3Qgd2FudCB0 byBjbG9iYmVyIHRoZSBjdXJyZW50CiAgICAgLy8gaGlzdG9yeSBpdGVtIGFzIHRoaXMgY291bGQg Y2F1c2UgdXMgdG8gbG9zZSB0aGUgc2Nyb2xsIHBvc2l0aW9uIGFuZAogICAgIC8vIGRvY3VtZW50 IHN0YXRlLiAgSG93ZXZlciwgaXQgaXMgT0sgZm9yIG5ldyBuYXZpZ2F0aW9ucy4KLSAgICBpZiAo bV9mcmFtZS0+bG9hZGVyKCktPmxvYWRUeXBlKCkgPT0gRnJhbWVMb2FkVHlwZVN0YW5kYXJkCi0g ICAgICAgIHx8ICFtX2ZyYW1lLT5sb2FkZXIoKS0+YWN0aXZlRG9jdW1lbnRMb2FkZXIoKS0+aXNM b2FkaW5nSW5BUElTZW5zZSgpKQorICAgIGlmICghbV9pblNhbWVEb2N1bWVudEhpc3RvcnlMb2Fk ICYmIChtX2ZyYW1lLT5sb2FkZXIoKS0+bG9hZFR5cGUoKSA9PSBGcmFtZUxvYWRUeXBlU3RhbmRh cmQKKyAgICAgICAgfHwgIW1fZnJhbWUtPmxvYWRlcigpLT5hY3RpdmVEb2N1bWVudExvYWRlcigp LT5pc0xvYWRpbmdJbkFQSVNlbnNlKCkpKQogICAgICAgICBtX2ZyYW1lLT5sb2FkZXIoKS0+aGlz dG9yeSgpLT5zYXZlRG9jdW1lbnRBbmRTY3JvbGxTdGF0ZSgpOwogCiAgICAgcmV0dXJuIFdlYkhp c3RvcnlJdGVtKG1fZnJhbWUtPnBhZ2UoKS0+YmFja0ZvcndhcmQoKS0+Y3VycmVudEl0ZW0oKSk7 CkBAIC0xOTI4LDYgKzE5MzAsNyBAQAogICAgICwgbV9uZXh0SW52YWxpZGF0ZUFmdGVyKDApCiAg ICAgLCBtX2FuaW1hdGlvbkNvbnRyb2xsZXIodGhpcykKICAgICAsIG1faWRlbnRpZmllcihnZW5l cmF0ZUZyYW1lSWRlbnRpZmllcigpKQorICAgICwgbV9pblNhbWVEb2N1bWVudEhpc3RvcnlMb2Fk KGZhbHNlKQogewogICAgIFBsYXRmb3JtQnJpZGdlOjppbmNyZW1lbnRTdGF0c0NvdW50ZXIod2Vi RnJhbWVBY3RpdmVDb3VudCk7CiAgICAgZnJhbWVDb3VudCsrOwpJbmRleDogU291cmNlL1dlYktp dC9jaHJvbWl1bS9zcmMvV2ViRnJhbWVJbXBsLmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYktp dC9jaHJvbWl1bS9zcmMvV2ViRnJhbWVJbXBsLmgJKHJldmlzaW9uIDg1NzY2KQorKysgU291cmNl L1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViRnJhbWVJbXBsLmgJKHdvcmtpbmcgY29weSkKQEAgLTQx Miw2ICs0MTIsMTAgQEAKIAogICAgIC8vIFRoZSBpZGVudGlmaWVyIG9mIHRoaXMgZnJhbWUuCiAg ICAgbG9uZyBsb25nIG1faWRlbnRpZmllcjsKKworICAgIC8vIEVuc3VyZSB3ZSBkb24ndCBvdmVy d3JpdGUgdmFsaWQgaGlzdG9yeSBkYXRhIGR1cmluZyBzYW1lIGRvY3VtZW50IGxvYWRzCisgICAg Ly8gZnJvbSBIaXN0b3J5SXRlbXMKKyAgICBib29sIG1faW5TYW1lRG9jdW1lbnRIaXN0b3J5TG9h ZDsKIH07CiAKIH0gLy8gbmFtZXNwYWNlIFdlYktpdApJbmRleDogU291cmNlL1dlYktpdC9jaHJv bWl1bS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYktpdC9jaHJvbWl1bS9DaGFu Z2VMb2cJKHJldmlzaW9uIDg1Nzg0KQorKysgU291cmNlL1dlYktpdC9jaHJvbWl1bS9DaGFuZ2VM b2cJKHdvcmtpbmcgY29weSkKQEAgLTEsMyArMSwxNyBAQAorMjAxMS0wNS0wNCAgTmF0ZSBDaGFw aW4gIDxqYXBoZXRAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgIEVuc3VyZSB3ZSBkb24ndCBvdmVyd3JpdGUgdXNlci1zcGVjaWZp ZWQgc2Nyb2xsIHBvc2l0aW9ucworICAgICAgICBvbiBzYW1lIGRvY3VtZW50IEhpc3RvcnlJdGVt IG5hdmlnYXRpb25zLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9NTk4NzcKKworICAgICAgICAqIHNyYy9XZWJGcmFtZUltcGwuY3BwOgorICAgICAgICAo V2ViS2l0OjpXZWJGcmFtZUltcGw6OmxvYWRIaXN0b3J5SXRlbSk6CisgICAgICAgIChXZWJLaXQ6 OldlYkZyYW1lSW1wbDo6Y3VycmVudEhpc3RvcnlJdGVtKToKKyAgICAgICAgKFdlYktpdDo6V2Vi RnJhbWVJbXBsOjpXZWJGcmFtZUltcGwpOgorICAgICAgICAqIHNyYy9XZWJGcmFtZUltcGwuaDoK KwogMjAxMS0wNS0wNCAgSmF5IENpdmVsbGkgIDxqY2l2ZWxsaUBjaHJvbWl1bS5vcmc+CiAKICAg ICAgICAgUmV2aWV3ZWQgYnkgT2phbiBWYWZhaS4K