194934 2019-02-21 23:02:46 -0800 Spurious find results on many apple.com pages 2022-02-09 10:13:49 -0800 1 1 1 Unclassified WebKit WebCore Misc. WebKit Nightly Build Unspecified Unspecified NEW InRadar P2 Normal --- 1 thorton thorton ap bdakin darin jonlee koivisto rniwa simon.fraser webkit-bug-importer wenson_hsieh zalan oldest_to_newest 1508903 0 thorton 2019-02-21 23:02:46 -0800 Apple.com seems to have adopted a technique where they hide accessibility text all over the page in spans with this style: clip: rect(1px, 1px, 1px, 1px); -webkit-clip-path: inset(0px 0px 99.9% 99.9%); clip-path: inset(0px 0px 99.9% 99.9%); overflow: hidden; height: 1px; width: 1px; If you use find-in-page to search for text inside of one of these elements, we happily return it as a potential result, though the text is not visible, and the find hole and highlight end up being 1x1 px. I'm not sure the best way or place to detect this, but making TextIterator's `fullyClipsContents` return true if the contentSize's area is <= 1px certainly fixes it (and makes find on these pages feel much more sensible). 1508904 1 thorton 2019-02-21 23:03:08 -0800 Steps to Reproduce: 1. Search for 'apple' or 'iphone' on apple.com 1509023 2 darin 2019-02-22 09:52:52 -0800 The heuristic about very small sizes makes sense to me and seems worth doing rather than a lot of soul searching. I think we could consider a threshold even larger than 1px, based on some assumptions about the smallest size for readable text. 1509024 3 darin 2019-02-22 09:53:43 -0800 What is not so clear to me is which part of this is the key. The small size, the clipping. It would be nice to have the heuristic consider the minimum clue necessary to help ensure it works on the widest number of pages. 1509156 4 rniwa 2019-02-22 13:43:06 -0800 Hm.. maybe 2-3px box would be too small? I think anything bigger than that, we run the risk of things being visible when zoomed, or the author intentionally showing really tiny text. 1544785 5 thorton 2019-06-14 11:27:18 -0700 <rdar://problem/51739857> 1544810 6 372136 thorton 2019-06-14 13:00:31 -0700 Created attachment 372136 Patch 1544821 7 thorton 2019-06-14 13:28:25 -0700 Hilariously, the overlay test failures are legit, because that test puts its log content in a "position: absolute; height: 1px; width: 1px; overflow: hidden;" div (for some reason), and now TextIterator decides not to dump that text when dumpAsText-ing the layout test. Not really sure we can change global TextIterator behavior that much (Darin? Ryosuke? smfr is not on board anymore) 1544823 8 thorton 2019-06-14 13:29:51 -0700 The other option is to add a TextIteratorOption that find uses, but it seems weiiiird for plainText() and find to include different text. 1544868 9 darin 2019-06-14 15:27:46 -0700 (In reply to Tim Horton from comment #7) > Hilariously, the overlay test failures are legit, because that test puts its > log content in a "position: absolute; height: 1px; width: 1px; overflow: > hidden;" div (for some reason), and now TextIterator decides not to dump > that text when dumpAsText-ing the layout test. > > Not really sure we can change global TextIterator behavior that much (Darin? > Ryosuke? smfr is not on board anymore) I think this kind of change is OK. I can’t think of any concrete use of TextIterator where iterating this invisible text is a plus. We should change those tests to not depend on this. 1544871 10 darin 2019-06-14 15:31:47 -0700 (In reply to Tim Horton from comment #8) > The other option is to add a TextIteratorOption that find uses, but it seems > weiiiird for plainText() and find to include different text. We need to go through the different TextIterator use types. There are probably still fewer than 10. I doubt we can find any where it’s a benefit to include this kind of invisible text. The use of TextIterator in dumpAsText is probably the least "legit" use of it. It's basically designed for use in Copy and Find, and then we found many uses for that in text editing as well. If there’s someone arguing that such invisible text *should* continue to be made visible when you copy and paste, I suggest that’s what we should discuss, not the abstract "what should a text iterator do". 1544889 11 rniwa 2019-06-14 16:15:01 -0700 TextIterator is also used to implement innerText for now, and that API's behavior shouldn't change. 1544891 12 thorton 2019-06-14 16:20:53 -0700 (In reply to Ryosuke Niwa from comment #11) > TextIterator is also used to implement innerText for now, and that API's > behavior shouldn't change. Hmm, I assumed that set TextIteratorIgnoresStyleVisibility but it does not. 1544962 13 koivisto 2019-06-14 23:08:21 -0700 Time for another flag! 1545045 14 darin 2019-06-15 13:02:32 -0700 Seems risky that innerText is possibly important for website compatibility but it’s not precisely specified. 1545075 15 darin 2019-06-15 18:18:12 -0700 OK, so if innerText does not set TextIteratorIgnoresStyleVisibility: 1) I think it should. 2) I think then that changing to treat 1x1 text as invisible is a good idea anyway. 3) But why would any of these need to be web-visible. I do not want to hold off on fixing this bug forever just because of innerText! 372136 2019-06-14 13:00:31 -0700 2019-06-14 13:29:59 -0700 Patch bug-194934-20190614130030.patch text/plain 3105 thorton U3VidmVyc2lvbiBSZXZpc2lvbjogMjQ2NDM5CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMGFhNmQwODdjMjY4ODMz NGJjN2VhMmE0NjM5ODA0ZmFmNTUyN2U5Zi4uMjY4N2FlYmQyYWI0MWEwZGY0NGM2ZDAwZmIwM2Y0 NTJjMzExZGEzOSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDE5LTA2LTE0ICBUaW0g SG9ydG9uICA8dGltb3RoeV9ob3J0b25AYXBwbGUuY29tPgorCisgICAgICAgIFNwdXJpb3VzIGZp bmQgcmVzdWx0cyBvbiBtYW55IGFwcGxlLmNvbSBwYWdlcworICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTk0OTM0CisgICAgICAgIDxyZGFyOi8vcHJvYmxl bS81MTczOTg1Nz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAg ICAgICBOZXcgQVBJIHRlc3Q6IFdlYktpdC5Eb05vdEZpbmRNb3N0bHlDbGlwcGVkT3V0VGV4dC4K KworICAgICAgICAqIGVkaXRpbmcvVGV4dEl0ZXJhdG9yLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6 OmZ1bGx5Q2xpcHNDb250ZW50cyk6CisgICAgICAgIEhhdmUgVGV4dEl0ZXJhdG9yIGNvbnNpZGVy IGEgMXgxIGNsaXAgYXMgImZ1bGx5IGNsaXBwZWQgb3V0IiB0ZXh0LgorCiAyMDE5LTA2LTE0ICBT YWFtIEJhcmF0aSAgPHNiYXJhdGlAYXBwbGUuY29tPgogCiAgICAgICAgIFVucmV2aWV3ZWQuIEZv bGxvdyB1cCB0byByMjQ2NDM4LiBUaGlzIHJlbW92ZXMgYSBkZWJ1ZyBhc3NlcnQgdW50aWwKZGlm ZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2VkaXRpbmcvVGV4dEl0ZXJhdG9yLmNwcCBiL1NvdXJj ZS9XZWJDb3JlL2VkaXRpbmcvVGV4dEl0ZXJhdG9yLmNwcAppbmRleCAyMzQxNGEyOTU2NDQwMGU4 NDdjNDUxNWJhMjlhOTY5ZWU4ZGIyYTgxLi43YTk4MzM5YTUzNzcxYjliMzcxMzRmOGFkNTQwYTNk ZDUzNDNlNGFiIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9lZGl0aW5nL1RleHRJdGVyYXRv ci5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvZWRpdGluZy9UZXh0SXRlcmF0b3IuY3BwCkBAIC0y MjAsNyArMjIwLDcgQEAgc3RhdGljIGlubGluZSBib29sIGZ1bGx5Q2xpcHNDb250ZW50cyhOb2Rl JiBub2RlKQogICAgIGlmIChpczxIVE1MVGV4dEFyZWFFbGVtZW50Pihub2RlKSkKICAgICAgICAg cmV0dXJuIGJveC5zaXplKCkuaXNFbXB0eSgpOwogCi0gICAgcmV0dXJuIGJveC5jb250ZW50U2l6 ZSgpLmlzRW1wdHkoKTsKKyAgICByZXR1cm4gRmxvYXRTaXplKGJveC5jb250ZW50U2l6ZSgpKS5h cmVhKCkgPD0gMTsKIH0KIAogc3RhdGljIGlubGluZSBib29sIGlnbm9yZXNDb250YWluZXJDbGlw KE5vZGUmIG5vZGUpCmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cgYi9Ub29scy9DaGFuZ2VM b2cKaW5kZXggZTNhYTRlOTM3OWI3MGI3NGI3MjZjYTk5MzBjZjM1NGFhZWY5YjdjZS4uMzE2NWE0 MzQ5MmU3MWZjY2Q1NDBlNTI1NzUyMmQ5ZDAzN2IzMGQ1OSAxMDA2NDQKLS0tIGEvVG9vbHMvQ2hh bmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE0IEBACisyMDE5LTA2LTE0 ICBUaW0gSG9ydG9uICA8dGltb3RoeV9ob3J0b25AYXBwbGUuY29tPgorCisgICAgICAgIFNwdXJp b3VzIGZpbmQgcmVzdWx0cyBvbiBtYW55IGFwcGxlLmNvbSBwYWdlcworICAgICAgICBodHRwczov L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTk0OTM0CisgICAgICAgIDxyZGFyOi8v cHJvYmxlbS81MTczOTg1Nz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICAqIFRlc3RXZWJLaXRBUEkvVGVzdHMvV2ViS2l0Q29jb2EvRmluZEluUGFnZS5t bToKKyAgICAgICAgKFRFU1QpOgorCiAyMDE5LTA2LTE0ICBZb3Vlbm4gRmFibGV0ICA8eW91ZW5u QGFwcGxlLmNvbT4KIAogICAgICAgICBpbXBvcnQtdzNjLXRlc3RzIHNob3VsZCByZXNwZWN0IFdF QktJVF9PVVRQVVRESVIKZGlmZiAtLWdpdCBhL1Rvb2xzL1Rlc3RXZWJLaXRBUEkvVGVzdHMvV2Vi S2l0Q29jb2EvRmluZEluUGFnZS5tbSBiL1Rvb2xzL1Rlc3RXZWJLaXRBUEkvVGVzdHMvV2ViS2l0 Q29jb2EvRmluZEluUGFnZS5tbQppbmRleCA0MmQwNzZjNDUzODFjNTc3MTFkN2M1MWY3ODAxMmY2 NDlhZWY4NzVmLi5iY2QxZTczZThmYjZjZWEyNTZiZDZmMTEzOTZlMDM5MThiMDdlMjY5IDEwMDY0 NAotLS0gYS9Ub29scy9UZXN0V2ViS2l0QVBJL1Rlc3RzL1dlYktpdENvY29hL0ZpbmRJblBhZ2Uu bW0KKysrIGIvVG9vbHMvVGVzdFdlYktpdEFQSS9UZXN0cy9XZWJLaXRDb2NvYS9GaW5kSW5QYWdl Lm1tCkBAIC0yNDgsNCArMjQ4LDE1IEBAIFRFU1QoV2ViS2l0LCBGaW5kQW5kUmVwbGFjZSkKICAg ICBFWFBFQ1RfV0tfU1RSRVEoImhpIGhpIiwgW3dlYlZpZXcgc3RyaW5nQnlFdmFsdWF0aW5nSmF2 YVNjcmlwdDpAImRvY3VtZW50LmJvZHkudGV4dENvbnRlbnQiXSk7CiB9CiAKK1RFU1QoV2ViS2l0 LCBEb05vdEZpbmRNb3N0bHlDbGlwcGVkT3V0VGV4dCkKK3sKKyAgICBSZXRhaW5QdHI8VGVzdFdL V2ViVmlldz4gd2ViVmlldyA9IGFkb3B0TlMoW1tUZXN0V0tXZWJWaWV3IGFsbG9jXSBpbml0V2l0 aEZyYW1lOk5TTWFrZVJlY3QoMCwgMCwgMjAwLCAyMDApXSk7CisKKyAgICBbd2ViVmlldyBzeW5j aHJvbm91c2x5TG9hZEhUTUxTdHJpbmc6QCI8ZGl2IHN0eWxlPSd3aWR0aDogMXB4OyBoZWlnaHQ6 IDFweDsgb3ZlcmZsb3c6IGhpZGRlbjsnPkJpcnRoZGF5PC9kaXY+PGRpdj5CaXJ0aGRheTwvZGl2 PiJdOworCisgICAgLy8gU2hvdWxkIG9ubHkgZmluZCBvbmUgbWF0Y2g7IHRoZSBmaXJzdCBvbmUg aXMgY2xpcHBlZCBvdXQgZW5vdWdoIHRoYXQgaXQgZG9lc24ndCBjb3VudC4KKyAgICBhdXRvIHJl c3VsdCA9IGZpbmRNYXRjaGVzKHdlYlZpZXcuZ2V0KCksIEAiQmlydGhkYXkiKTsKKyAgICBFWFBF Q1RfRVEoKE5TVUludGVnZXIpMSwgW3Jlc3VsdC5tYXRjaGVzIGNvdW50XSk7Cit9CisKICNlbmRp ZiAvLyAhUExBVEZPUk0oSU9TX0ZBTUlMWSkK