181255 2018-01-03 12:30:40 -0800 Web Inspector: Find banner sometimes does not work (when already populated and shown for first time on resource) 2018-01-03 16:46:57 -0800 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 joepeck joepeck commit-queue inspector-bugzilla-changes joepeck mattbaker webkit-bug-importer oldest_to_newest 1385539 0 joepeck 2018-01-03 12:30:40 -0800 Find banner sometimes does not work (when already populated and shown for first time on resource) Steps to Reproduce: 1. Inspect webkit.org 2. Show Debugger tab 3. Select "global.js" 4. Show Find Banner (⌘F) 5. Search for "document" => 3 results 6. Reload the page (⌘R) 7. Select global.js again if needed 8. Show Find Banner (⌘F) => Already populated with "document" 9. Hit Enter => No results Notes: - ContentBrowser owns the FindBanner and applies it to whatever ContentView it is showing - When a ContentBrowser opens a TextContentView* for Resource A (1) TextContentView triggers Load Content of Resource A => NetworkAgent.getResponseBody / Debugger.getScriptSource (2) ContentBrowser triggers Perform Search with existing FindBanner value => Page.searchInResource / Debugger.searchInContent While these execute on the backend serially, the result for getting the content body goes through multiple microtask loops. So (1) might be processed in the TextEditor after we attempt to process (2). 1385540 1 joepeck 2018-01-03 12:30:48 -0800 <rdar://problem/36248855> 1385543 2 joepeck 2018-01-03 12:36:37 -0800 ContentBrowser just does: 1. Open ContentView 2. ContentView.performSearch(query) And here, text based ContentViews does asynchronous work in both of these: 1.1 Load Content from backend 2.1 Search Content on backend --- So really, this can be a problem with automatically performing a search on any content view that loads its content asynchronously. In practice I think that is really only the content views containing a TextEditor, and those proxy the searching to TextEditor. So for now, I'm just going to make TextEditor defer the search until it has loaded its initial content. Ensuring that when it does the search it should be able to show the results. This also might have a minor perf win to avoid performing searches on resources every time you select them, even if the find banner is not visible. --- We also appear to have an issue where ⌘E / ⌘G doesn't work unless the FindBanner is visible, which will be a separate bug. 1385546 3 330411 joepeck 2018-01-03 12:41:06 -0800 Created attachment 330411 [PATCH] Proposed Fix 1385608 4 330411 mattbaker 2018-01-03 16:41:33 -0800 Comment on attachment 330411 [PATCH] Proposed Fix r=me 1385616 5 330411 commit-queue 2018-01-03 16:46:56 -0800 Comment on attachment 330411 [PATCH] Proposed Fix Clearing flags on attachment: 330411 Committed r226380: <https://trac.webkit.org/changeset/226380> 1385617 6 commit-queue 2018-01-03 16:46:57 -0800 All reviewed patches have been landed. Closing bug. 330411 2018-01-03 12:41:06 -0800 2018-01-03 16:46:56 -0800 [PATCH] Proposed Fix search-fix-1.patch text/plain 2777 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCmluZGV4IDdlN2JkNjIwZWI2Li4yZGJmN2IxZjUzMSAxMDA2 NDQKLS0tIGEvU291cmNlL1dlYkluc3BlY3RvclVJL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCkBAIC0xLDUgKzEsMjIgQEAKIDIwMTgtMDEtMDMgIEpvc2Vw aCBQZWNvcmFybyAgPHBlY29yYXJvQGFwcGxlLmNvbT4KIAorICAgICAgICBXZWIgSW5zcGVjdG9y OiBGaW5kIGJhbm5lciBzb21ldGltZXMgZG9lcyBub3Qgd29yayAod2hlbiBhbHJlYWR5IHBvcHVs YXRlZCBhbmQgc2hvd24gZm9yIGZpcnN0IHRpbWUgb24gcmVzb3VyY2UpCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xODEyNTUKKyAgICAgICAgPHJkYXI6 Ly9wcm9ibGVtLzM2MjQ4ODU1PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgICogVXNlckludGVyZmFjZS9WaWV3cy9UZXh0RWRpdG9yLmpzOgorICAgICAg ICAoV0kuVGV4dEVkaXRvci5wcm90b3R5cGUuc2V0IHN0cmluZyk6CisgICAgICAgIERlZmVyIGFu eSBlYXJseSBzZWFyY2hlcyB1bnRpbCB0aGUgaW5pdGlhbCBjb250ZW50IG9mIGEgVGV4dEVkaXRv ciBoYXMgYmVlbiBzZXQuCisgICAgICAgIFN1Y2ggc2VhcmNoZXMgY2FuIGhhcHBlbiB3aGVuIHRo ZSBGaW5kQmFubmVyIGFscmVhZHkgaGFzIGNvbnRlbnQgd2hlbiBhCisgICAgICAgIENvbnRlbnRW aWV3IGlzIGZpcnN0IG9wZW5lZCBhbmQgbmVlZHMgdG8gbG9hZCBpdHMgY29udGVudCBmcm9tIHRo ZSBiYWNrZW5kLgorICAgICAgICBGdXJ0aGVyLCBldmVuIHRob3VnaCB0aGUgY29udGVudCBtYXkg YmUgbG9hZGVkIGZyb20gdGhlIGJhY2tlbmQgYmVmb3JlIHRoZQorICAgICAgICBzZWFyY2ggcmVz dWx0cywgbWljcm90YXNrIGhvcHMgbWlnaHQgY2F1c2UgdGhlIGNvbnRlbnQgdG8gZ2V0IHRvIHRo ZSBUZXh0RWRpdG9yCisgICAgICAgIGFmdGVyIHRoZSBzZWFyY2ggcmVzdWx0cy4KKworMjAxOC0w MS0wMyAgSm9zZXBoIFBlY29yYXJvICA8cGVjb3Jhcm9AYXBwbGUuY29tPgorCiAgICAgICAgIFJF R1JFU1NJT046IFdlYiBJbnNwZWN0b3I6IERlYnVnZ2VyIHRhYiBkb2Vzbid0IHJlc3RvcmUgc2Vs ZWN0ZWQgcmVzb3VyY2Ugb24gcmVsb2FkCiAgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD0xODEyNTMKICAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzM2MjgwNTY0 PgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvVmlld3Mv VGV4dEVkaXRvci5qcyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdz L1RleHRFZGl0b3IuanMKaW5kZXggMzA3NTQzMjg0YzMuLjBlNzQyYzcyZmU5IDEwMDY0NAotLS0g YS9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9UZXh0RWRpdG9yLmpz CisrKyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL1RleHRFZGl0 b3IuanMKQEAgLTEyOSw3ICsxMjksMTUgQEAgV0kuVGV4dEVkaXRvciA9IGNsYXNzIFRleHRFZGl0 b3IgZXh0ZW5kcyBXSS5WaWV3CiAgICAgICAgICAgICBpZiAodGhpcy5faW5pdGlhbFN0cmluZ05v dFNldCkgewogICAgICAgICAgICAgICAgIHRoaXMuX2NvZGVNaXJyb3IuY2xlYXJIaXN0b3J5KCk7 CiAgICAgICAgICAgICAgICAgdGhpcy5fY29kZU1pcnJvci5tYXJrQ2xlYW4oKTsKKwogICAgICAg ICAgICAgICAgIHRoaXMuX2luaXRpYWxTdHJpbmdOb3RTZXQgPSBmYWxzZTsKKworICAgICAgICAg ICAgICAgIC8vIFRoZXJlIG1heSBoYXZlIGJlZW4gYW4gYXR0ZW1wdCBhdCBhIHNlYXJjaCBiZWZv cmUgdGhlIGluaXRpYWwgc3RyaW5nIHdhcyBzZXQuIElmIHNvLCByZXBlcmZvcm0gaXQgbm93IHRo YXQgd2UgaGF2ZSBjb250ZW50LgorICAgICAgICAgICAgICAgIGlmICh0aGlzLl9zZWFyY2hRdWVy eSkgeworICAgICAgICAgICAgICAgICAgICBsZXQgcXVlcnkgPSB0aGlzLl9zZWFyY2hRdWVyeTsK KyAgICAgICAgICAgICAgICAgICAgdGhpcy5fc2VhcmNoUXVlcnkgPSBudWxsOworICAgICAgICAg ICAgICAgICAgICB0aGlzLnBlcmZvcm1TZWFyY2gocXVlcnkpOworICAgICAgICAgICAgICAgIH0K ICAgICAgICAgICAgIH0KIAogICAgICAgICAgICAgLy8gVXBkYXRlIHRoZSBleGVjdXRpb24gbGlu ZSBub3cgdGhhdCB3ZSBtaWdodCBoYXZlIGNvbnRlbnQgZm9yIHRoYXQgbGluZS4KQEAgLTI5NCw2 ICszMDIsMTAgQEAgV0kuVGV4dEVkaXRvciA9IGNsYXNzIFRleHRFZGl0b3IgZXh0ZW5kcyBXSS5W aWV3CiAKICAgICAgICAgdGhpcy5fc2VhcmNoUXVlcnkgPSBxdWVyeTsKIAorICAgICAgICAvLyBE ZWZlciB1bnRpbCB0aGUgaW5pdGlhbCBzdHJpbmcgaXMgc2V0LgorICAgICAgICBpZiAodGhpcy5f aW5pdGlhbFN0cmluZ05vdFNldCkKKyAgICAgICAgICAgIHJldHVybjsKKwogICAgICAgICAvLyBB bGxvdyBzdWJjbGFzc2VzIHRvIGhhbmRsZSB0aGUgc2VhcmNoaW5nIGlmIHRoZXkgaGF2ZSBhIGJl dHRlciB3YXkuCiAgICAgICAgIC8vIElmIHdlIGFyZSBmb3JtYXR0ZWQsIGp1c3QgdXNlIENvZGVN aXJyb3IncyBzZWFyY2guCiAgICAgICAgIGlmICh0eXBlb2YgdGhpcy5jdXN0b21QZXJmb3JtU2Vh cmNoID09PSAiZnVuY3Rpb24iICYmICF0aGlzLmZvcm1hdHRlZCkgewo=