132239 2014-04-27 15:20:23 -0700 REGRESSION (r164702): Double tap doesn't stay under the new element once the animation finishes 2014-04-27 22:40:01 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 thorton thorton benjamin sam simon.fraser oldest_to_newest 1004443 0 thorton 2014-04-27 15:20:23 -0700 The early return added to WebPage::scalePage is wrong because the origin can be different (and it only checks the scale), and also because it doesn't consult the PluginView's page scale. <rdar://problem/16192842> 1004444 1 230270 thorton 2014-04-27 15:24:23 -0700 Created attachment 230270 patch 1004447 2 thorton 2014-04-27 15:48:31 -0700 http://trac.webkit.org/changeset/167864 1004457 3 benjamin 2014-04-27 18:44:31 -0700 This fixes OS X but breaks iOS. :( 1004458 4 benjamin 2014-04-27 18:54:08 -0700 Ok, this breaks animated resize and the initial scale logic on iOS. The patch is okay for OS X, we should split this for iOS IMHO. 1004464 5 thorton 2014-04-27 19:34:33 -0700 (In reply to comment #4) > Ok, this breaks animated resize and the initial scale logic on iOS. > > The patch is okay for OS X, we should split this for iOS IMHO. What kind of split are you proposing? Also, does iOS just disregard the origin parameter? Do we need to just additionally not do the dynamic size update/scaleFactorWasSetFromUIProcess bits if the scale doesn't change? 1004468 6 benjamin 2014-04-27 20:00:47 -0700 (In reply to comment #5) > (In reply to comment #4) > > Ok, this breaks animated resize and the initial scale logic on iOS. > > > > The patch is okay for OS X, we should split this for iOS IMHO. > > What kind of split are you proposing? See https://bugs.webkit.org/show_bug.cgi?id=126022 It looks like WebPage::scalePage() and Page::setPageScaleFactor() do not work well when the scroll position is not handled by the ScrollView. > Also, does iOS just disregard the origin parameter? Yep, iOS uses delegateScrolling(), and Page::setPageScaleFactor() must ignore the scroll position or we will re-enter WebPage. Simon fixed that a while ago. > Do we need to just additionally not do the dynamic size update/scaleFactorWasSetFromUIProcess bits if the scale doesn't change? Yep, short term we should do that to unbreak iOS. Longer term we should look into having a single coherent path for setting the scroll position from WebCore, in the same way that we have the coherent path for scale+scroll update from the UIProcess. 1004479 7 thorton 2014-04-27 20:48:04 -0700 (In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > Ok, this breaks animated resize and the initial scale logic on iOS. > > > > > > The patch is okay for OS X, we should split this for iOS IMHO. > > > > What kind of split are you proposing? > > See https://bugs.webkit.org/show_bug.cgi?id=126022 > It looks like WebPage::scalePage() and Page::setPageScaleFactor() do not work well when the scroll position is not handled by the ScrollView. Hmmpfh. Ok. > > Also, does iOS just disregard the origin parameter? > > Yep, iOS uses delegateScrolling(), and Page::setPageScaleFactor() must ignore the scroll position or we will re-enter WebPage. Simon fixed that a while ago. > > > Do we need to just additionally not do the dynamic size update/scaleFactorWasSetFromUIProcess bits if the scale doesn't change? > > Yep, short term we should do that to unbreak iOS. > Longer term we should look into having a single coherent path for setting the scroll position from WebCore, in the same way that we have the coherent path for scale+scroll update from the UIProcess. OK, I'll post a patch to just do that to unbreak iOS, for now. Reopening for that. 1004480 8 230277 thorton 2014-04-27 20:54:57 -0700 Created attachment 230277 followup 1004481 9 thorton 2014-04-27 21:08:13 -0700 http://trac.webkit.org/changeset/167869 1004488 10 benjamin 2014-04-27 22:40:01 -0700 Thanks! 230270 2014-04-27 15:24:23 -0700 2014-04-27 20:54:57 -0700 patch scale.diff text/plain 2034 thorton ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCBkOTgwYTcxLi5mYmRjNDMwIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjEg QEAKKzIwMTQtMDQtMjcgIFRpbSBIb3J0b24gIDx0aW1vdGh5X2hvcnRvbkBhcHBsZS5jb20+CisK KyAgICAgICAgUkVHUkVTU0lPTiAocjE2NDcwMik6IERvdWJsZSB0YXAgZG9lc24ndCBzdGF5IHVu ZGVyIHRoZSBuZXcgZWxlbWVudCBvbmNlIHRoZSBhbmltYXRpb24gZmluaXNoZXMKKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEzMjIzOQorICAgICAgICA8 cmRhcjovL3Byb2JsZW0vMTYxOTI4NDI+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChP T1BTISkuCisKKyAgICAgICAgKiBXZWJQcm9jZXNzL1dlYlBhZ2UvV2ViUGFnZS5jcHA6CisgICAg ICAgIChXZWJLaXQ6OldlYlBhZ2U6OnNjYWxlUGFnZSk6CisgICAgICAgIFRoZSBlYXJseS1yZXR1 cm4gYWRkZWQgdG8gV2ViUGFnZTo6c2NhbGVQYWdlIGJyZWFrcyBjYWxsZXJzIHdobyBkZXBlbmQK KyAgICAgICAgb24gYmVpbmcgYWJsZSB0byBjYWxsIHNjYWxlUGFnZSgpIHdpdGggdGhlIHNhbWUg c2NhbGUgYnV0IGEgZGlmZmVyZW50CisgICAgICAgIG9yaWdpbiBhbmQgaGF2aW5nIHRoYXQgY2hh bmdlIHRha2UgZWZmZWN0LgorCisgICAgICAgIFBhZ2U6OnNldFBhZ2VTY2FsZUZhY3RvciBhbHJl YWR5IGhhcyB0aGUgcmVxdWlzaXRlIGxvZ2ljLCBzbyBtb3ZlCisgICAgICAgIHRoZSBlYXJseSBy ZXR1cm4gZG93biBhZnRlciB0aGF0IGNhbGwsIGFuZCBndWFyZCBvbmx5IG5vdGlmaWNhdGlvbgor ICAgICAgICBvZiBwYWdlIHNjYWxlIGNoYW5nZXMuCisKIDIwMTQtMDQtMjcgIFByYXRpayBTb2xh bmtpICA8cHNvbGFua2lAYXBwbGUuY29tPgogCiAgICAgICAgIFVucmV2aWV3ZWQuIGlPUyBidWls ZCBmaXguCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1dlYlBhZ2UvV2Vi UGFnZS5jcHAgYi9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1dlYlBhZ2UvV2ViUGFnZS5jcHAK aW5kZXggZDdhMDgzYi4uOGFmYjc3YiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvV2ViUHJv Y2Vzcy9XZWJQYWdlL1dlYlBhZ2UuY3BwCisrKyBiL1NvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3Mv V2ViUGFnZS9XZWJQYWdlLmNwcApAQCAtMTI4OCw5ICsxMjg4LDYgQEAgdm9pZCBXZWJQYWdlOjp3 aW5kb3dTY3JlZW5EaWRDaGFuZ2UodWludDY0X3QgZGlzcGxheUlEKQogCiB2b2lkIFdlYlBhZ2U6 OnNjYWxlUGFnZShkb3VibGUgc2NhbGUsIGNvbnN0IEludFBvaW50JiBvcmlnaW4pCiB7Ci0gICAg aWYgKHNjYWxlID09IHBhZ2VTY2FsZUZhY3RvcigpKQotICAgICAgICByZXR1cm47Ci0KICNpZiBQ TEFURk9STShJT1MpCiAgICAgaWYgKCFtX2luRHluYW1pY1NpemVVcGRhdGUpCiAgICAgICAgIG1f ZHluYW1pY1NpemVVcGRhdGVIaXN0b3J5LmNsZWFyKCk7CkBAIC0xMzAyLDggKzEyOTksMTQgQEAg dm9pZCBXZWJQYWdlOjpzY2FsZVBhZ2UoZG91YmxlIHNjYWxlLCBjb25zdCBJbnRQb2ludCYgb3Jp Z2luKQogICAgICAgICByZXR1cm47CiAgICAgfQogCisgICAgZmxvYXQgb2xkUGFnZVNjYWxlRmFj dG9yID0gcGFnZVNjYWxlRmFjdG9yKCk7CisKICAgICBtX3BhZ2UtPnNldFBhZ2VTY2FsZUZhY3Rv cihzY2FsZSwgb3JpZ2luKTsKIAorICAgIC8vIFdlIGNhbid0IGVhcmx5IHJldHVybiBiZWZvcmUg c2V0UGFnZVNjYWxlRmFjdG9yIGJlY2F1c2UgdGhlIG9yaWdpbiBtaWdodCBiZSBkaWZmZXJlbnQu CisgICAgaWYgKHNjYWxlID09IG9sZFBhZ2VTY2FsZUZhY3RvcikKKyAgICAgICAgcmV0dXJuOwor CiAgICAgZm9yIChhdXRvKiBwbHVnaW5WaWV3IDogbV9wbHVnaW5WaWV3cykKICAgICAgICAgcGx1 Z2luVmlldy0+cGFnZVNjYWxlRmFjdG9yRGlkQ2hhbmdlKCk7CiAK 230277 2014-04-27 20:54:57 -0700 2014-04-27 20:56:31 -0700 followup scale.diff text/plain 2397 thorton ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCAyNmRmNjk5Li5kMzIyZTI2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTgg QEAKKzIwMTQtMDQtMjcgIFRpbSBIb3J0b24gIDx0aW1vdGh5X2hvcnRvbkBhcHBsZS5jb20+CisK KyAgICAgICAgUkVHUkVTU0lPTiAocjE2NDcwMik6IERvdWJsZSB0YXAgZG9lc24ndCBzdGF5IHVu ZGVyIHRoZSBuZXcgZWxlbWVudCBvbmNlIHRoZSBhbmltYXRpb24gZmluaXNoZXMKKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEzMjIzOQorICAgICAgICA8 cmRhcjovL3Byb2JsZW0vMTYxOTI4NDI+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChP T1BTISkuCisKKyAgICAgICAgKiBXZWJQcm9jZXNzL1dlYlBhZ2UvV2ViUGFnZS5jcHA6CisgICAg ICAgIChXZWJLaXQ6OldlYlBhZ2U6OnNjYWxlUGFnZSk6CisgICAgICAgIFRoZSBjaGFuZ2UgaW4g cjE2Nzg2NCBicm9rZSBpT1MgYW5pbWF0ZWQgcmVzaXplLCBiZWNhdXNlIGl0IHdhcyBkZXBlbmRp bmcgb24KKyAgICAgICAgdGhlIGR5bmFtaWMgc2l6ZSB1cGRhdGUgY29kZSBub3QgcnVubmluZyBp ZiB0aGUgc2NhbGUgd2Fzbid0IGdvaW5nIHRvIGNoYW5nZS4KKyAgICAgICAgU28sIGFzIGEgYmFu ZC1haWQgd2Ugc2hvdWxkIGJhaWwgZnJvbSBkb2luZyB0aGF0IHdvcmsgaWYgdGhlIHNjYWxlcyBh cmVuJ3QgZGlmZmVyZW50LgorICAgICAgICBJbiB0aGUgbG9uZyB0ZXJtIHdlIHNob3VsZCB0cnkg dG8gdW50YW5nbGUgdGhpcyBjb2RlIGFuZCBtYWtlIGl0IGxlc3MgcGxhdGZvcm0gZGVwZW5kZW50 LgorCiAyMDE0LTA0LTI3ICBFdW5taSBMZWUgIDxldW5taTE1LmxlZUBzYW1zdW5nLmNvbT4KIAog ICAgICAgICBUb3VjaEV2ZW50IGlzIG5vdCBoYW5kbGVkIGFmdGVyIHJlbGVhc2luZyBhbnkgcG9p bnQgYW1vbmcgdG91Y2hlZCBwb2ludHMuCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9XZWJQ cm9jZXNzL1dlYlBhZ2UvV2ViUGFnZS5jcHAgYi9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1dl YlBhZ2UvV2ViUGFnZS5jcHAKaW5kZXggZGU0MjE5OC4uNDZkMjkzNyAxMDA2NDQKLS0tIGEvU291 cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJQYWdlL1dlYlBhZ2UuY3BwCisrKyBiL1NvdXJjZS9X ZWJLaXQyL1dlYlByb2Nlc3MvV2ViUGFnZS9XZWJQYWdlLmNwcApAQCAtMTI5MSwxMCArMTI5MSwx NCBAQCB2b2lkIFdlYlBhZ2U6OndpbmRvd1NjcmVlbkRpZENoYW5nZSh1aW50NjRfdCBkaXNwbGF5 SUQpCiAKIHZvaWQgV2ViUGFnZTo6c2NhbGVQYWdlKGRvdWJsZSBzY2FsZSwgY29uc3QgSW50UG9p bnQmIG9yaWdpbikKIHsKKyAgICBib29sIHdpbGxDaGFuZ2VTY2FsZUZhY3RvciA9IHNjYWxlICE9 IHBhZ2VTY2FsZUZhY3RvcigpOworCiAjaWYgUExBVEZPUk0oSU9TKQotICAgIGlmICghbV9pbkR5 bmFtaWNTaXplVXBkYXRlKQotICAgICAgICBtX2R5bmFtaWNTaXplVXBkYXRlSGlzdG9yeS5jbGVh cigpOwotICAgIG1fc2NhbGVXYXNTZXRCeVVJUHJvY2VzcyA9IGZhbHNlOworICAgIGlmICh3aWxs Q2hhbmdlU2NhbGVGYWN0b3IpIHsKKyAgICAgICAgaWYgKCFtX2luRHluYW1pY1NpemVVcGRhdGUp CisgICAgICAgICAgICBtX2R5bmFtaWNTaXplVXBkYXRlSGlzdG9yeS5jbGVhcigpOworICAgICAg ICBtX3NjYWxlV2FzU2V0QnlVSVByb2Nlc3MgPSBmYWxzZTsKKyAgICB9CiAjZW5kaWYKICAgICBQ bHVnaW5WaWV3KiBwbHVnaW5WaWV3ID0gcGx1Z2luVmlld0ZvckZyYW1lKCZtX3BhZ2UtPm1haW5G cmFtZSgpKTsKICAgICBpZiAocGx1Z2luVmlldyAmJiBwbHVnaW5WaWV3LT5oYW5kbGVzUGFnZVNj YWxlRmFjdG9yKCkpIHsKQEAgLTEzMDIsMTIgKzEzMDYsMTAgQEAgdm9pZCBXZWJQYWdlOjpzY2Fs ZVBhZ2UoZG91YmxlIHNjYWxlLCBjb25zdCBJbnRQb2ludCYgb3JpZ2luKQogICAgICAgICByZXR1 cm47CiAgICAgfQogCi0gICAgZmxvYXQgb2xkUGFnZVNjYWxlRmFjdG9yID0gcGFnZVNjYWxlRmFj dG9yKCk7Ci0KICAgICBtX3BhZ2UtPnNldFBhZ2VTY2FsZUZhY3RvcihzY2FsZSwgb3JpZ2luKTsK IAogICAgIC8vIFdlIGNhbid0IGVhcmx5IHJldHVybiBiZWZvcmUgc2V0UGFnZVNjYWxlRmFjdG9y IGJlY2F1c2UgdGhlIG9yaWdpbiBtaWdodCBiZSBkaWZmZXJlbnQuCi0gICAgaWYgKHNjYWxlID09 IG9sZFBhZ2VTY2FsZUZhY3RvcikKKyAgICBpZiAoIXdpbGxDaGFuZ2VTY2FsZUZhY3RvcikKICAg ICAgICAgcmV0dXJuOwogCiAgICAgZm9yIChhdXRvKiBwbHVnaW5WaWV3IDogbV9wbHVnaW5WaWV3 cykK