139946 2014-12-25 09:41:31 -0800 Fix typo in StyleResolveTree.cpp 2016-03-11 13:47:04 -0800 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) iPhone / iPad Unspecified RESOLVED FIXED P2 Minor --- 120173 1 mkisilev bfulgham bfulgham commit-queue dbates mkisilev oldest_to_newest 1057840 0 mkisilev 2014-12-25 09:41:31 -0800 In constructor of class CheckForVisibilityChangeOnRecalcStyle there is a line: , m_previousImplicitVisibility(WKObservingContentChanges() && WKContentChange() != WKContentVisibilityChange ? elementImplicitVisibility(element) : VISIBLE) Calling WKContentChange() will produce enum of type WKContentChange whose value will alwayse be not equal to WKContentVisibilityChange. Probably author wanted to call WKObservedContentChange() instead of WKContentChange(). Line should be replace with: , m_previousImplicitVisibility(WKObservingContentChanges() && WKObservedContentChange() != WKContentVisibilityChange ? elementImplicitVisibility(element) : VISIBLE) 1057841 1 243745 mkisilev 2014-12-25 09:54:46 -0800 Created attachment 243745 Patch 1057842 2 commit-queue 2014-12-25 09:57:26 -0800 Attachment 243745 did not pass style-queue: ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style. 1057843 3 243746 mkisilev 2014-12-25 10:05:19 -0800 Created attachment 243746 Patch 1057845 4 243746 ap 2014-12-25 10:53:56 -0800 Comment on attachment 243746 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=243746&action=review > Source/WebCore/ChangeLog:8 > + No new tests. Behaviour wasn't changed. If there is no change in behavior, what is the rationale for making this code change? 1057858 5 mkisilev 2014-12-25 13:23:16 -0800 (In reply to comment #4) > Comment on attachment 243746 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=243746&action=review > > > Source/WebCore/ChangeLog:8 > > + No new tests. Behaviour wasn't changed. > > If there is no change in behavior, what is the rationale for making this > code change? Final behavior wasn't changed. But code before change had meaningless check for: WKContentChange() != WKContentVisibilityChange 1173164 6 bfulgham 2016-03-10 11:02:04 -0800 (In reply to comment #5) > (In reply to comment #4) > > Comment on attachment 243746 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=243746&action=review > > > > > Source/WebCore/ChangeLog:8 > > > + No new tests. Behaviour wasn't changed. > > > > If there is no change in behavior, what is the rationale for making this > > code change? > > Final behavior wasn't changed. But code before change had meaningless check > for: > WKContentChange() != WKContentVisibilityChange Yes, this code is wrong. WKContentChange() is not a function, it's a type -- so this is like saying "int() != WKContentVisibilityChange" The correct method is WKObservedContentChange(). I'm not sure what impact this has in practice, but it should be corrected. 1173176 7 273584 bfulgham 2016-03-10 11:16:58 -0800 Created attachment 273584 Patch 1173177 8 bfulgham 2016-03-10 11:17:40 -0800 I rebaselined Maksim's patch. Let's see what EWS says... 1173503 9 bfulgham 2016-03-10 17:59:49 -0800 EWS was happy with this change. 1173505 10 273584 aestes 2016-03-10 18:07:36 -0800 Comment on attachment 273584 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=273584&action=review > Source/WebCore/ChangeLog:8 > + No new tests. Behaviour wasn't changed. You state below that you actually expect some change in behavior, so I don't think you can claim nothing changed. 1173795 11 bfulgham 2016-03-11 10:59:49 -0800 I'm trying to see if I can figure out a test case for this. This code is only used on iOS, and only triggers in the mouseMoved and 'handleSyntheticClick" event handler. Businessinsider.com helpfully generates synthetic clicks for you, which shows the code getting hit. With the bug, we always use the "elementImplicitVisibility" path. This means that an element might stay hidden that (according to this code) should have been forced to be VISIBLE. 1173810 12 dbates 2016-03-11 11:39:34 -0800 (In reply to comment #11) > I'm trying to see if I can figure out a test case for this. > You may find inspiration from looking at the tests included in <http://trac.webkit.org/changeset/194038> (bug #144451). In particular, see test <http://trac.webkit.org/browser/trunk/LayoutTests/fast/events/can-click-element-on-page-with-active-pseudo-class-and-search-field.html>. We may have additional tests that exercise the CheckForVisibilityChangeOnRecalcStyle code in <http://trac.webkit.org/browser/trunk/LayoutTests/platform/ios-simulator/ios>. 1173850 13 bfulgham 2016-03-11 13:47:04 -0800 Committed r198044: <http://trac.webkit.org/changeset/198044> 243745 2014-12-25 09:54:46 -0800 2014-12-25 10:05:12 -0800 Patch bug-139946-20141225195447.patch text/plain 1661 mkisilev U3VidmVyc2lvbiBSZXZpc2lvbjogMTc3NzM1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMmU2ZDE5MWJhOTE0MDIx YjM2Yzk1YjcxMGM4NWYzOTFhMTZmYmE2ZC4uYTk5MjQ1NzdjNzNhMjA5ZjBiOWE1Yjg2YzhlYmQ1 YTBmZDE5ZDBkZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDE0LTEyLTI1ICBNYWtz aW0gS2lzaWxldiAgPG1raXNpbGV2QHlhbmRleC10ZWFtLnJ1PgorCisgICAgICAgIEZpeCB0eXBv IGluIFN0eWxlUmVzb2x2ZVRyZWUuY3BwCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD0xMzk5NDYKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KKworICAgICAgICBObyBuZXcgdGVzdHMgKE9PUFMhKS4KKworICAgICAgICAqIHN0eWxl L1N0eWxlUmVzb2x2ZVRyZWUuY3BwOgorICAgICAgICAoV2ViQ29yZTo6U3R5bGU6OkNoZWNrRm9y VmlzaWJpbGl0eUNoYW5nZU9uUmVjYWxjU3R5bGU6OkNoZWNrRm9yVmlzaWJpbGl0eUNoYW5nZU9u UmVjYWxjU3R5bGUpOgorCiAyMDE0LTEyLTI1ICBDb21taXQgUXVldWUgIDxjb21taXQtcXVldWVA d2Via2l0Lm9yZz4KIAogICAgICAgICBVbnJldmlld2VkLCByb2xsaW5nIG91dCByMTc3NzEyIGFu ZCByMTc3NzE3LgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvc3R5bGUvU3R5bGVSZXNvbHZl VHJlZS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9zdHlsZS9TdHlsZVJlc29sdmVUcmVlLmNwcAppbmRl eCBiODhkN2YzMmJjNmFlNjgxYTNlNzJjYmMzZGIwYTRiYTBlNzA1OGVhLi5lY2QzMWZkOWFmOTJl YTQxMTgzOTQ4ZjM3YTFlZTY2ZTQzMDcyYmQ3IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9z dHlsZS9TdHlsZVJlc29sdmVUcmVlLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9zdHlsZS9TdHls ZVJlc29sdmVUcmVlLmNwcApAQCAtODcwLDcgKzg3MCw3IEBAIHB1YmxpYzoKICAgICAgICAgOiBt X2VsZW1lbnQoZWxlbWVudCkKICAgICAgICAgLCBtX3ByZXZpb3VzRGlzcGxheShjdXJyZW50U3R5 bGUgPyBjdXJyZW50U3R5bGUtPmRpc3BsYXkoKSA6IE5PTkUpCiAgICAgICAgICwgbV9wcmV2aW91 c1Zpc2liaWxpdHkoY3VycmVudFN0eWxlID8gY3VycmVudFN0eWxlLT52aXNpYmlsaXR5KCkgOiBI SURERU4pCi0gICAgICAgICwgbV9wcmV2aW91c0ltcGxpY2l0VmlzaWJpbGl0eShXS09ic2Vydmlu Z0NvbnRlbnRDaGFuZ2VzKCkgJiYgV0tDb250ZW50Q2hhbmdlKCkgIT0gV0tDb250ZW50VmlzaWJp bGl0eUNoYW5nZSA/IGVsZW1lbnRJbXBsaWNpdFZpc2liaWxpdHkoZWxlbWVudCkgOiBWSVNJQkxF KQorICAgICAgICAsIG1fcHJldmlvdXNJbXBsaWNpdFZpc2liaWxpdHkoV0tPYnNlcnZpbmdDb250 ZW50Q2hhbmdlcygpICYmIFdLT2JzZXJ2ZWRDb250ZW50Q2hhbmdlKCkgIT0gV0tDb250ZW50Vmlz aWJpbGl0eUNoYW5nZSA/IGVsZW1lbnRJbXBsaWNpdFZpc2liaWxpdHkoZWxlbWVudCkgOiBWSVNJ QkxFKQogICAgIHsKICAgICB9CiAgICAgfkNoZWNrRm9yVmlzaWJpbGl0eUNoYW5nZU9uUmVjYWxj U3R5bGUoKQo= 243746 2014-12-25 10:05:19 -0800 2016-03-10 11:16:51 -0800 Patch bug-139946-20141225200520.patch text/plain 1679 mkisilev U3VidmVyc2lvbiBSZXZpc2lvbjogMTc3NzM1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMmU2ZDE5MWJhOTE0MDIx YjM2Yzk1YjcxMGM4NWYzOTFhMTZmYmE2ZC4uNzZhYTY2OGExZjc5MGNiOGM5YzMwYzA5NmJmODU4 YmY0Mzk3MTY5MCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDE0LTEyLTI1ICBNYWtz aW0gS2lzaWxldiAgPG1raXNpbGV2QHlhbmRleC10ZWFtLnJ1PgorCisgICAgICAgIEZpeCB0eXBv IGluIFN0eWxlUmVzb2x2ZVRyZWUuY3BwCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD0xMzk5NDYKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KKworICAgICAgICBObyBuZXcgdGVzdHMuIEJlaGF2aW91ciB3YXNuJ3QgY2hhbmdlZC4K KworICAgICAgICAqIHN0eWxlL1N0eWxlUmVzb2x2ZVRyZWUuY3BwOgorICAgICAgICAoV2ViQ29y ZTo6U3R5bGU6OkNoZWNrRm9yVmlzaWJpbGl0eUNoYW5nZU9uUmVjYWxjU3R5bGU6OkNoZWNrRm9y VmlzaWJpbGl0eUNoYW5nZU9uUmVjYWxjU3R5bGUpOgorCiAyMDE0LTEyLTI1ICBDb21taXQgUXVl dWUgIDxjb21taXQtcXVldWVAd2Via2l0Lm9yZz4KIAogICAgICAgICBVbnJldmlld2VkLCByb2xs aW5nIG91dCByMTc3NzEyIGFuZCByMTc3NzE3LgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUv c3R5bGUvU3R5bGVSZXNvbHZlVHJlZS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9zdHlsZS9TdHlsZVJl c29sdmVUcmVlLmNwcAppbmRleCBiODhkN2YzMmJjNmFlNjgxYTNlNzJjYmMzZGIwYTRiYTBlNzA1 OGVhLi5lY2QzMWZkOWFmOTJlYTQxMTgzOTQ4ZjM3YTFlZTY2ZTQzMDcyYmQ3IDEwMDY0NAotLS0g YS9Tb3VyY2UvV2ViQ29yZS9zdHlsZS9TdHlsZVJlc29sdmVUcmVlLmNwcAorKysgYi9Tb3VyY2Uv V2ViQ29yZS9zdHlsZS9TdHlsZVJlc29sdmVUcmVlLmNwcApAQCAtODcwLDcgKzg3MCw3IEBAIHB1 YmxpYzoKICAgICAgICAgOiBtX2VsZW1lbnQoZWxlbWVudCkKICAgICAgICAgLCBtX3ByZXZpb3Vz RGlzcGxheShjdXJyZW50U3R5bGUgPyBjdXJyZW50U3R5bGUtPmRpc3BsYXkoKSA6IE5PTkUpCiAg ICAgICAgICwgbV9wcmV2aW91c1Zpc2liaWxpdHkoY3VycmVudFN0eWxlID8gY3VycmVudFN0eWxl LT52aXNpYmlsaXR5KCkgOiBISURERU4pCi0gICAgICAgICwgbV9wcmV2aW91c0ltcGxpY2l0Vmlz aWJpbGl0eShXS09ic2VydmluZ0NvbnRlbnRDaGFuZ2VzKCkgJiYgV0tDb250ZW50Q2hhbmdlKCkg IT0gV0tDb250ZW50VmlzaWJpbGl0eUNoYW5nZSA/IGVsZW1lbnRJbXBsaWNpdFZpc2liaWxpdHko ZWxlbWVudCkgOiBWSVNJQkxFKQorICAgICAgICAsIG1fcHJldmlvdXNJbXBsaWNpdFZpc2liaWxp dHkoV0tPYnNlcnZpbmdDb250ZW50Q2hhbmdlcygpICYmIFdLT2JzZXJ2ZWRDb250ZW50Q2hhbmdl KCkgIT0gV0tDb250ZW50VmlzaWJpbGl0eUNoYW5nZSA/IGVsZW1lbnRJbXBsaWNpdFZpc2liaWxp dHkoZWxlbWVudCkgOiBWSVNJQkxFKQogICAgIHsKICAgICB9CiAgICAgfkNoZWNrRm9yVmlzaWJp bGl0eUNoYW5nZU9uUmVjYWxjU3R5bGUoKQo= 273584 2016-03-10 11:16:58 -0800 2016-03-10 18:03:45 -0800 Patch bug-139946-20160310111645.patch text/plain 2186 bfulgham SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE5Nzk0NSkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI0IEBACisyMDE2LTAzLTEwICBNYWtzaW0g S2lzaWxldiA8bWtpc2lsZXZAeWFuZGV4LXRlYW0ucnU+CisKKyAgICAgICAgRml4IHR5cG8gaW4g U3R5bGVUcmVlUmVzb2x2ZXIuY3BwCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD0xMzk5NDYKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICBObyBuZXcgdGVzdHMuIEJlaGF2aW91ciB3YXNuJ3QgY2hhbmdlZC4KKwor ICAgICAgICBUaGUgY29uc3RydWN0b3IgZm9yIENoZWNrRm9yVmlzaWJpbGl0eUNoYW5nZU9uUmVj YWxjU3R5bGUgd2FzIGltcHJvcGVybHkgY29tcGFyaW5nIHRoZQorICAgICAgICByZXN1bHQgb2Yg V0tDb250ZW50Q2hhbmdlKCkgKHdoaWNoIGlzIG5vdCBhIGZ1bmN0aW9uKSB0byBXS0NvbnRlbnRW aXNpYmlsaXR5Q2hhbmdlLiBJCisgICAgICAgIGJlbGlldmUgdGhlIGFib3ZlIGNhc3Qgd291bGQg aW1wbGljaXRseSByZXNvbHZlIHRvIFdLQ29udGVudE5vQ2hhbmdlIGluIGFsbCBjYXNlcywKKyAg ICAgICAgd2hldGhlciBhIHZpc2liaWxpdHkgY2hhbmdlIGhhZCBiZWVuIG9ic2VydmVkIG9yIG5v dC4KKyAgICAgICAgCisgICAgICAgIFRoaXMgcGF0Y2ggY29ycmVjdHMgdGhpcyBwcm9ibGVtLiBJ IHdvdWxkIGV4cGVjdCB0aGF0IHRoaXMgbWlnaHQgYWZmZWN0IHNvbWUgY29udGVudAorICAgICAg ICB2aXNpYmlsaXR5IGNoYW5nZSBiZWhhdmlvciwgYnV0IEknbSBub3Qgc3VyZSB3aGF0IHRoZSBh cHByb3ByaWF0ZSB0ZXN0IGNhc2Ugd291bGQgYmUKKyAgICAgICAgc2luY2UgdGhpcyB3YXMgYXBw YXJlbnRseSBmb3VuZCB0aHJvdWdoIGNvZGUgaW5zcGVjdGlvbi4KKworICAgICAgICAqIHN0eWxl L1N0eWxlVHJlZVJlc29sdmVyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlN0eWxlOjpDaGVja0Zv clZpc2liaWxpdHlDaGFuZ2VPblJlY2FsY1N0eWxlOjpDaGVja0ZvclZpc2liaWxpdHlDaGFuZ2VP blJlY2FsY1N0eWxlKToKKwogMjAxNi0wMy0xMCAgRGFuaWVsIEJhdGVzICA8ZGFiYXRlc0BhcHBs ZS5jb20+CiAKICAgICAgICAgQ1NQOiBJbXBsZW1lbnQgc3VwcG9ydCBmb3Igc2NyaXB0IGFuZCBz dHlsZSBub25jZXMKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL3N0eWxlL1N0eWxlVHJlZVJlc29sdmVy LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9zdHlsZS9TdHlsZVRyZWVSZXNvbHZl ci5jcHAJKHJldmlzaW9uIDE5NzkyMCkKKysrIFNvdXJjZS9XZWJDb3JlL3N0eWxlL1N0eWxlVHJl ZVJlc29sdmVyLmNwcAkod29ya2luZyBjb3B5KQpAQCAtNzkwLDcgKzc5MCw3IEBAIHB1YmxpYzoK ICAgICAgICAgOiBtX2VsZW1lbnQoZWxlbWVudCkKICAgICAgICAgLCBtX3ByZXZpb3VzRGlzcGxh eShjdXJyZW50U3R5bGUgPyBjdXJyZW50U3R5bGUtPmRpc3BsYXkoKSA6IE5PTkUpCiAgICAgICAg ICwgbV9wcmV2aW91c1Zpc2liaWxpdHkoY3VycmVudFN0eWxlID8gY3VycmVudFN0eWxlLT52aXNp YmlsaXR5KCkgOiBISURERU4pCi0gICAgICAgICwgbV9wcmV2aW91c0ltcGxpY2l0VmlzaWJpbGl0 eShXS09ic2VydmluZ0NvbnRlbnRDaGFuZ2VzKCkgJiYgV0tDb250ZW50Q2hhbmdlKCkgIT0gV0tD b250ZW50VmlzaWJpbGl0eUNoYW5nZSA/IGVsZW1lbnRJbXBsaWNpdFZpc2liaWxpdHkoZWxlbWVu dCkgOiBWSVNJQkxFKQorICAgICAgICAsIG1fcHJldmlvdXNJbXBsaWNpdFZpc2liaWxpdHkoV0tP YnNlcnZpbmdDb250ZW50Q2hhbmdlcygpICYmIFdLT2JzZXJ2ZWRDb250ZW50Q2hhbmdlKCkgIT0g V0tDb250ZW50VmlzaWJpbGl0eUNoYW5nZSA/IGVsZW1lbnRJbXBsaWNpdFZpc2liaWxpdHkoZWxl bWVudCkgOiBWSVNJQkxFKQogICAgIHsKICAgICB9CiAgICAgfkNoZWNrRm9yVmlzaWJpbGl0eUNo YW5nZU9uUmVjYWxjU3R5bGUoKQo=