138198 2014-10-29 16:57:59 -0700 Web Inspector: TextEditor search doesn't work after editing contents 2014-11-03 17:03:42 -0800 1 1 1 Unclassified WebKit Web Inspector 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 joepeck joepeck commit-queue graouts joepeck timothy webkit-bug-importer oldest_to_newest 1044908 0 joepeck 2014-10-29 16:57:59 -0700 * SUMMARY TextEditor search doesn't work after editing a file. Stops early. Wrong number of results. * STEPS TO REPRODUCE 1. Inspect <http://bogojoker.com/shell/> 2. Select Stylesheet Resource "print.css" 3. Search for "font" => 10 results 4. Add a blank link at line 29 5. Clear search, and search for "font" again => 8 results stopping the count at the edit, expected 10 results. * NOTES SourceCodeTextEditor is using its backend search instead of frontend (CodeMirror) search when the editor has edits. 1044909 1 webkit-bug-importer 2014-10-29 16:58:11 -0700 <rdar://problem/18818999> 1044910 2 240638 joepeck 2014-10-29 17:03:12 -0700 Created attachment 240638 [PATCH] Proposed Fix 1044911 3 joepeck 2014-10-29 17:04:24 -0700 Heck if we want maybe we should always have CodeMirror perform searching inside of TextEditor. is there a reason we might not want that? 1045794 4 240638 timothy 2014-11-03 11:29:51 -0800 Comment on attachment 240638 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=240638&action=review > Source/WebInspectorUI/UserInterface/Views/TextEditor.js:232 > + get hasEdits() This is something I would make a function. The rule I follow: if it is a question or action, it is a function. Otherwise, a property. > Source/WebInspectorUI/UserInterface/Views/TextEditor.js:241 > + var undoableChanges = this._codeMirror.historySize().undo; > + if (!undoableChanges) > + return false; > + if (undoableChanges > 1) > + return true; > + > + // Don't claim that we have edits if we have only one change which was pretty-printing. > + return !this._formatted; I wonder if we should use this._codeMirror.isClean() here? Does it give the right result? Maybe we need some well placed markClean() calls to make that work though. (We do a markClean in TextEditor.set string().) 1045811 5 240638 joepeck 2014-11-03 11:54:50 -0800 Comment on attachment 240638 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=240638&action=review >> Source/WebInspectorUI/UserInterface/Views/TextEditor.js:232 >> + get hasEdits() > > This is something I would make a function. The rule I follow: if it is a question or action, it is a function. Otherwise, a property. Agreed, I'll make the change. >> Source/WebInspectorUI/UserInterface/Views/TextEditor.js:241 >> + return !this._formatted; > > I wonder if we should use this._codeMirror.isClean() here? Does it give the right result? Maybe we need some well placed markClean() calls to make that work though. (We do a markClean in TextEditor.set string().) Oh nice, I had forgotten about that. That would certainly be more convenient. I'll give that a try. 1045862 6 240877 joepeck 2014-11-03 14:43:04 -0800 Created attachment 240877 [PATCH] Proposed Fix 1045865 7 240877 timothy 2014-11-03 14:50:13 -0800 Comment on attachment 240877 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=240877&action=review > Source/WebInspectorUI/UserInterface/Views/TextEditor.js:818 > + return !this._codeMirror.isClean(); The previous patch accounted for pretty-printing, does that still need to happen? 1045893 8 240877 joepeck 2014-11-03 16:26:34 -0800 Comment on attachment 240877 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=240877&action=review >> Source/WebInspectorUI/UserInterface/Views/TextEditor.js:818 >> + return !this._codeMirror.isClean(); > > The previous patch accounted for pretty-printing, does that still need to happen? Turns out we already don't use the customPerformSearch if the content is formatted. Right now, hasEdits() returns false if the editor has been pretty printed. I think that is fine unless we want to tweak that later. 1045902 9 240877 commit-queue 2014-11-03 17:03:37 -0800 Comment on attachment 240877 [PATCH] Proposed Fix Clearing flags on attachment: 240877 Committed r175497: <http://trac.webkit.org/changeset/175497> 1045903 10 commit-queue 2014-11-03 17:03:42 -0800 All reviewed patches have been landed. Closing bug. 240638 2014-10-29 17:03:12 -0700 2014-11-03 14:43:04 -0800 [PATCH] Proposed Fix search-after-edits.patch text/plain 2630 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCmluZGV4IGM4NjZlYjYuLjZlMjgwNGMgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkluc3BlY3Rv clVJL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDIyIEBACiAyMDE0LTEwLTI5ICBKb3NlcGggUGVjb3Jh cm8gIDxwZWNvcmFyb0BhcHBsZS5jb20+CiAKKyAgICAgICAgV2ViIEluc3BlY3RvcjogVGV4dEVk aXRvciBzZWFyY2ggZG9lc24ndCB3b3JrIGFmdGVyIGVkaXRpbmcgY29udGVudHMKKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEzODE5OAorCisgICAgICAg IFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogVXNlckludGVyZmFjZS9W aWV3cy9Tb3VyY2VDb2RlVGV4dEVkaXRvci5qczoKKyAgICAgICAgKFdlYkluc3BlY3Rvci5Tb3Vy Y2VDb2RlVGV4dEVkaXRvci5wcm90b3R5cGUuY3VzdG9tUGVyZm9ybVNlYXJjaCk6CisgICAgICAg IEZhbGxiYWNrIHRvIHB1cmUgVGV4dEVkaXRvciBjb250ZW50IHNlYXJjaGluZyAoQ29kZU1pcnJv cikgaWYgdGhlIGVkaXRvciBoYXMgZWRpdHMuCisKKyAgICAgICAgKiBVc2VySW50ZXJmYWNlL1Zp ZXdzL1RleHRFZGl0b3IuanM6CisgICAgICAgIChXZWJJbnNwZWN0b3IuVGV4dEVkaXRvci5wcm90 b3R5cGUuZ2V0IGhhc0VkaXRzKToKKyAgICAgICAgQ2xhaW0gd2UgaGF2ZSBlZGl0cyB3aGVuIHdl IGhhdmUgYW4gdW5kbyBzdGFjay4gU3BlY2lhbCBjYXNlIGlmIHdlCisgICAgICAgIGhhdmUgYW4g dW5kbyBzdGFjayBvZiBvbmUgYXMgYSByZXN1bHQgb2YgcHJldHR5LXByaW50aW5nLCBhcyB3ZSBj YW4KKyAgICAgICAgc3RpbGwgaGFuZGxlIHRoYXQgcGVyZmVjdGx5LgorCisyMDE0LTEwLTI5ICBK b3NlcGggUGVjb3Jhcm8gIDxwZWNvcmFyb0BhcHBsZS5jb20+CisKICAgICAgICAgV2ViIEluc3Bl Y3RvcjogU2hvdyBTZWxlY3RvcidzIFNwZWNpZmljaXR5CiAgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMzgxODkKIApkaWZmIC0tZ2l0IGEvU291cmNlL1dl Ykluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvVmlld3MvU291cmNlQ29kZVRleHRFZGl0b3IuanMg Yi9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9Tb3VyY2VDb2RlVGV4 dEVkaXRvci5qcwppbmRleCA1ZjEwZmQ3Li45ODhjODhhIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9Tb3VyY2VDb2RlVGV4dEVkaXRvci5qcwor KysgYi9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9Tb3VyY2VDb2Rl VGV4dEVkaXRvci5qcwpAQCAtMjA2LDYgKzIwNiw5IEBAIFdlYkluc3BlY3Rvci5Tb3VyY2VDb2Rl VGV4dEVkaXRvci5wcm90b3R5cGUgPSB7CiAgICAgICAgICAgICB0aGlzLmRpc3BhdGNoRXZlbnRU b0xpc3RlbmVycyhXZWJJbnNwZWN0b3IuVGV4dEVkaXRvci5FdmVudC5OdW1iZXJPZlNlYXJjaFJl c3VsdHNEaWRDaGFuZ2UpOwogICAgICAgICB9CiAKKyAgICAgICAgaWYgKHRoaXMuaGFzRWRpdHMp CisgICAgICAgICAgICByZXR1cm4gZmFsc2U7CisKICAgICAgICAgaWYgKHRoaXMuX3NvdXJjZUNv ZGUgaW5zdGFuY2VvZiBXZWJJbnNwZWN0b3IuU291cmNlTWFwUmVzb3VyY2UpCiAgICAgICAgICAg ICByZXR1cm4gZmFsc2U7CiAKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2Vy SW50ZXJmYWNlL1ZpZXdzL1RleHRFZGl0b3IuanMgYi9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNl ckludGVyZmFjZS9WaWV3cy9UZXh0RWRpdG9yLmpzCmluZGV4IDA0ZGRiYWUuLjRkNjIwMGUgMTAw NjQ0Ci0tLSBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL1RleHRF ZGl0b3IuanMKKysrIGIvU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvVmlld3Mv VGV4dEVkaXRvci5qcwpAQCAtMjI5LDYgKzIyOSwxOCBAQCBXZWJJbnNwZWN0b3IuVGV4dEVkaXRv ci5wcm90b3R5cGUgPSB7CiAgICAgICAgIHRoaXMuX2NvZGVNaXJyb3Iuc2V0T3B0aW9uKCJtb2Rl IiwgbmV3TUlNRVR5cGUpOwogICAgIH0sCiAKKyAgICBnZXQgaGFzRWRpdHMoKQorICAgIHsKKyAg ICAgICAgdmFyIHVuZG9hYmxlQ2hhbmdlcyA9IHRoaXMuX2NvZGVNaXJyb3IuaGlzdG9yeVNpemUo KS51bmRvOworICAgICAgICBpZiAoIXVuZG9hYmxlQ2hhbmdlcykKKyAgICAgICAgICAgIHJldHVy biBmYWxzZTsKKyAgICAgICAgaWYgKHVuZG9hYmxlQ2hhbmdlcyA+IDEpCisgICAgICAgICAgICBy ZXR1cm4gdHJ1ZTsKKworICAgICAgICAvLyBEb24ndCBjbGFpbSB0aGF0IHdlIGhhdmUgZWRpdHMg aWYgd2UgaGF2ZSBvbmx5IG9uZSBjaGFuZ2Ugd2hpY2ggd2FzIHByZXR0eS1wcmludGluZy4KKyAg ICAgICAgcmV0dXJuICF0aGlzLl9mb3JtYXR0ZWQ7CisgICAgfSwKKwogICAgIGdldCBleGVjdXRp b25MaW5lTnVtYmVyKCkKICAgICB7CiAgICAgICAgIHJldHVybiB0aGlzLl9leGVjdXRpb25MaW5l TnVtYmVyOwo= 240877 2014-11-03 14:43:04 -0800 2014-11-03 17:03:37 -0800 [PATCH] Proposed Fix edits-2.patch text/plain 2271 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCmluZGV4IDk3NjU5ZmMuLmMzZWE1OGYgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkluc3BlY3Rv clVJL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDIwIEBACiAyMDE0LTExLTAzICBKb3NlcGggUGVjb3Jh cm8gIDxwZWNvcmFyb0BhcHBsZS5jb20+CiAKKyAgICAgICAgV2ViIEluc3BlY3RvcjogVGV4dEVk aXRvciBzZWFyY2ggZG9lc24ndCB3b3JrIGFmdGVyIGVkaXRpbmcgY29udGVudHMKKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEzODE5OAorCisgICAgICAg IFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogVXNlckludGVyZmFjZS9W aWV3cy9Tb3VyY2VDb2RlVGV4dEVkaXRvci5qczoKKyAgICAgICAgKFdlYkluc3BlY3Rvci5Tb3Vy Y2VDb2RlVGV4dEVkaXRvci5wcm90b3R5cGUuY3VzdG9tUGVyZm9ybVNlYXJjaCk6CisgICAgICAg IEZhbGxiYWNrIHRvIHB1cmUgVGV4dEVkaXRvciBjb250ZW50IHNlYXJjaGluZyAoQ29kZU1pcnJv cikgaWYgdGhlIGVkaXRvciBoYXMgZWRpdHMuCisKKyAgICAgICAgKiBVc2VySW50ZXJmYWNlL1Zp ZXdzL1RleHRFZGl0b3IuanM6CisgICAgICAgIChXZWJJbnNwZWN0b3IuVGV4dEVkaXRvci5wcm90 b3R5cGUuaGFzRWRpdHMpOgorICAgICAgICBDaGVjayB0aGUgQ29kZU1pcnJvciBjbGVhbiBzdGF0 ZSB0byBrbm93biBpZiB3ZSBoYXZlIGVkaXRzIG9yIG5vdC4KKworMjAxNC0xMS0wMyAgSm9zZXBo IFBlY29yYXJvICA8cGVjb3Jhcm9AYXBwbGUuY29tPgorCiAgICAgICAgIFdlYiBJbnNwZWN0b3I6 IEFMVEVSTkFURV9ESVNQQVRDSEVSUyBMZXQgdGhlIGZyb250ZW5kIGtub3cgYWJvdXQgZXh0cmEg YWdlbnRzCiAgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0x MzgyMzYKIApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2Uv Vmlld3MvU291cmNlQ29kZVRleHRFZGl0b3IuanMgYi9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNl ckludGVyZmFjZS9WaWV3cy9Tb3VyY2VDb2RlVGV4dEVkaXRvci5qcwppbmRleCA1ZjEwZmQ3Li44 NzVhNTVjIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9W aWV3cy9Tb3VyY2VDb2RlVGV4dEVkaXRvci5qcworKysgYi9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkv VXNlckludGVyZmFjZS9WaWV3cy9Tb3VyY2VDb2RlVGV4dEVkaXRvci5qcwpAQCAtMjA2LDYgKzIw Niw5IEBAIFdlYkluc3BlY3Rvci5Tb3VyY2VDb2RlVGV4dEVkaXRvci5wcm90b3R5cGUgPSB7CiAg ICAgICAgICAgICB0aGlzLmRpc3BhdGNoRXZlbnRUb0xpc3RlbmVycyhXZWJJbnNwZWN0b3IuVGV4 dEVkaXRvci5FdmVudC5OdW1iZXJPZlNlYXJjaFJlc3VsdHNEaWRDaGFuZ2UpOwogICAgICAgICB9 CiAKKyAgICAgICAgaWYgKHRoaXMuaGFzRWRpdHMoKSkKKyAgICAgICAgICAgIHJldHVybiBmYWxz ZTsKKwogICAgICAgICBpZiAodGhpcy5fc291cmNlQ29kZSBpbnN0YW5jZW9mIFdlYkluc3BlY3Rv ci5Tb3VyY2VNYXBSZXNvdXJjZSkKICAgICAgICAgICAgIHJldHVybiBmYWxzZTsKIApkaWZmIC0t Z2l0IGEvU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvVmlld3MvVGV4dEVkaXRv ci5qcyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL1RleHRFZGl0 b3IuanMKaW5kZXggMDRkZGJhZS4uZjQ0YmQzMiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkluc3Bl Y3RvclVJL1VzZXJJbnRlcmZhY2UvVmlld3MvVGV4dEVkaXRvci5qcworKysgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9UZXh0RWRpdG9yLmpzCkBAIC04MTMsNiAr ODEzLDExIEBAIFdlYkluc3BlY3Rvci5UZXh0RWRpdG9yLnByb3RvdHlwZSA9IHsKIAogICAgIC8v IFByaXZhdGUKIAorICAgIGhhc0VkaXRzOiBmdW5jdGlvbigpCisgICAgeworICAgICAgICByZXR1 cm4gIXRoaXMuX2NvZGVNaXJyb3IuaXNDbGVhbigpOworICAgIH0sCisKICAgICBfY29udGVudENo YW5nZWQ6IGZ1bmN0aW9uKGNvZGVNaXJyb3IsIGNoYW5nZSkKICAgICB7CiAgICAgICAgIGlmICh0 aGlzLl9pZ25vcmVDb2RlTWlycm9yQ29udGVudERpZENoYW5nZUV2ZW50ID4gMCkK