We need a matched substrings structure in UrlbarMatch objects so that the view can make them bold (or highlight them, or however it wants to handle them).

I started implementing this on UrlbarMatch, but I'm not sure that makes sense. It seems like this belongs in the view, for a couple of related reasons: (1) the view may not want to emphasize some matches or any matches at all, and (2) performance -- the model doesn't know which strings the view will want matches for. Just looking at the UnifiedComplete provider, there are many strings passed by the model to the view. It would be wasteful to do substring matching that the view isn't going to use. The overall point is that the model doesn't know how the view wants to show matching. So I implemented this directly in UrlbarView instead. It's easy to imagine other views besides the default one wanting to do matching. Doing matching in the model would solve that problem, but it's not a great idea for the reasons I mentioned above. So I can imagine moving the matching-related functions to a common view-utils module that all views could use.

Comment on attachment 9029378 [details] Bug 1511529 - Add substring matching/highlighting to UrlbarMatch and support it in UrlbarView. Marco, what do you think about this? (Please see the revision's commit message in comment 1.) I'll ask Dao for review if you're OK with it.

Or feel free to review yourself if you'd like, of course

(In reply to Drew Willcoxon :adw (Away 12/18–1/8) from comment #1) > The overall point is that the model doesn't know how the view wants to show > matching. How does the view know what the model used to match a result? For example, suppose that a search suggestion corrects a typo in the user's string, how does the view know to highlight the fixed-up string? Or if a WebExtension registers a magic keyword the view will have to only highlight the search terms. We may have future providers suggestions an article or news because the argument is "relevant" to the typed words even if they don't exactluy match those words, and it may want to hilight the synonym or related words. Additionally each view will have to reimplement matching for each match type, versus the model suggesting the view what it *may* want to highlight, and then the view just decides how/if to highlight. Not saying one solution is better than the other one, but it's probably worth evaluating what may better support our goal of a replace-able View and pluggable data sources. WDYT?

I spoke with Dao, we think that in the spite of supporting "content discovery" matches in the future that may want to hilight different words, it may be better to have hilight words in the providers. The performance problem may not be that horrible, we will only return a small amount of matches from the model, and the database/ferching work is much larger than some string comparison (where necessary), so it may not be measurable. We'd also be ok taking the implementation in the view for MVP and moving it in a future follow-up, depending on whether this blocks you implementing other stuff or whether you think it's not worth spending your time on that right now. But in general I think in the future we will need the added flexibility for providers to suggest what to highlight (obviously the view may decide to ignore those...).

OK, that makes sense.

Pushed by dwillcoxon@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b067a683472a Add substring matching/highlighting to UrlbarMatch and support it in UrlbarView. r=mak

Backed out changeset b067a683472a (bug 1511529) for xpcshell failure in browser/components/urlbar/tests/unit/test_providerOpenTabs.js. CLOSED TREE Log: [task 2018-12-11T20:17:51.242Z] 20:17:51 INFO - async*run_next_test/_run_next_test/<@/builds/worker/workspace/build/tests/xpcshell/head.js:1454:22 [task 2018-12-11T20:17:51.243Z] 20:17:51 INFO - async*_run_next_test@/builds/worker/workspace/build/tests/xpcshell/head.js:1454:10 [task 2018-12-11T20:17:51.243Z] 20:17:51 INFO - run@/builds/worker/workspace/build/tests/xpcshell/head.js:705:9 [task 2018-12-11T20:17:51.244Z] 20:17:51 INFO - _do_main@/builds/worker/workspace/build/tests/xpcshell/head.js:226:3 [task 2018-12-11T20:17:51.245Z] 20:17:51 INFO - _execute_test@/builds/worker/workspace/build/tests/xpcshell/head.js:546:5 [task 2018-12-11T20:17:51.245Z] 20:17:51 INFO - @-e:1:1 [task 2018-12-11T20:17:51.246Z] 20:17:51 INFO - exiting test [task 2018-12-11T20:17:51.247Z] 20:17:51 INFO - "CONSOLE_MESSAGE: (warn) [JavaScript Warning: "ReferenceError: reference to undefined property "tokens"" {file: "resource:///modules/UrlbarProviderOpenTabs.jsm" line: 168}]" [task 2018-12-11T20:17:51.247Z] 20:17:51 INFO - PID 11021 | [11021, Main Thread] WARNING: OOPDeinit() without successful OOPInit(): file /builds/worker/workspace/build/src/toolkit/crashreporter/nsExceptionHandler.cpp, line 3156 [task 2018-12-11T20:17:51.248Z] 20:17:51 INFO - PID 11021 | nsStringStats [task 2018-12-11T20:17:51.249Z] 20:17:51 INFO - PID 11021 | => mAllocCount: 7346 [task 2018-12-11T20:17:51.250Z] 20:17:51 INFO - PID 11021 | => mReallocCount: 0 [task 2018-12-11T20:17:51.251Z] 20:17:51 INFO - PID 11021 | => mFreeCount: 7346 [task 2018-12-11T20:17:51.251Z] 20:17:51 INFO - PID 11021 | => mShareCount: 11112 [task 2018-12-11T20:17:51.252Z] 20:17:51 INFO - PID 11021 | => mAdoptCount: 211 [task 2018-12-11T20:17:51.253Z] 20:17:51 INFO - PID 11021 | => mAdoptFreeCount: 211 [task 2018-12-11T20:17:51.254Z] 20:17:51 INFO - PID 11021 | => Process ID: 11021, Thread ID: 140346507723712 [task 2018-12-11T20:17:51.254Z] 20:17:51 INFO - <<<<<<< [task 2018-12-11T20:17:51.255Z] 20:17:51 INFO - INFO | Result summary: [task 2018-12-11T20:17:51.256Z] 20:17:51 INFO - INFO | Passed: 313 [task 2018-12-11T20:17:51.257Z] 20:17:51 WARNING - INFO | Failed: 1 [task 2018-12-11T20:17:51.257Z] 20:17:51 WARNING - One or more unittests failed. [task 2018-12-11T20:17:51.258Z] 20:17:51 INFO - INFO | Todo: 0 [task 2018-12-11T20:17:51.259Z] 20:17:51 INFO - INFO | Retried: 1 [task 2018-12-11T20:17:51.259Z] 20:17:51 INFO - SUITE-END | took 656s [task 2018-12-11T20:17:51.260Z] 20:17:51 INFO - Node moz-http2 server shutting down ... [task 2018-12-11T20:17:51.311Z] 20:17:51 ERROR - Return code: 1 [task 2018-12-11T20:17:51.312Z] 20:17:51 INFO - TinderboxPrint: xpcshell-xpcshell<br/>313/<em class="testfail">1</em>/0 [task 2018-12-11T20:17:51.313Z] 20:17:51 WARNING - # TBPL FAILURE # [task 2018-12-11T20:17:51.314Z] 20:17:51 WARNING - setting return code to 2 [task 2018-12-11T20:17:51.314Z] 20:17:51 WARNING - The xpcshell suite: xpcshell ran with return status: FAILURE Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=b067a683472aa977918ffd4d0d57a11630b515cc Backout: https://hg.mozilla.org/integration/autoland/rev/6ff605af4924543ff23791ea28f7d9e8459d66b1

Pushed by dwillcoxon@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a6c75a182585 Add substring matching/highlighting to UrlbarMatch and support it in UrlbarView. r=mak