[Spec] Re-write text matching steps

bokand commented 4 years ago

This commit is a re-write of the text matching portions of the spec.

It avoids using TreeWalker and is more specific and detailed in how the various algorithms work.

Fixes #73

:boom: Error: write EPROTO 139705468127104:error:1407742E:SSL routines:SSL23_GET_SERVER_HELLO:tlsv1 alert protocol version:../deps/openssl/openssl/ssl/s23_clnt.c:772:

:boom: ###

PR Preview failed to build. (Last tried on Mar 12, 2020, 8:46 PM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one: :rotating_light: [HTML Diff Service](http://services.w3.org/htmldiff) - The HTML Diff Service is used to create HTML diffs of the spec changes suggested in a pull request. :link: [Related URL](https://services.w3.org/htmldiff?doc1=https%3A%2F%2Fpr-preview.s3.amazonaws.com%2FWICG%2FScrollToTextFragment%2Fpull%2F90%2Fe770c01.html&doc2=https%3A%2F%2Fpr-preview.s3.amazonaws.com%2Fbokand%2FScrollToTextFragment%2Fpull%2F90.html) _If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please [file an issue](https://github.com/tobie/pr-preview/issues/new?title=Error%20not%20surfaced%20properly&body=See%20WICG/ScrollToTextFragment%2390.)._

bokand commented 4 years ago

@nickburris: It's a big change. I'll see if I can find a spec expert to review later but could you take a first pass?

Put up on https://bokand.github.io/spec.html for ease of review. I've pretty much rewritten all of section "3.5 Navigating to a Text Fragment" so you might just want the published copy here rather than a diff of the markdown.

bokand commented 4 years ago

Good idea, added some non-normative notes and an example

bokand commented 4 years ago

Thanks for the review. Note: there are still issues even with this update, two that are top of mind for me:

Using the raw CharacterData.data to perform the match (naively) is wrong because this is the raw character data which doesn't necessarily correspond to what's rendered. e.g. whitespace is uncollapsed in this form. (I'll investigate this and fix in a followup)
The specified word-bounding checks for word boundaries at the time when a string is searched in the concatenated string of all continuous nodes in a block container. This collapses inline nodes. However, Chrome's implementation considers inline nodes to be word bounds. (I think we should change Chrome's implementation to match the spec text).

Will address this and others in follow-ups

Edit: Also, the prefix/suffix as specified aren't compatible with RTL bug

bokand commented 4 years ago

@domenic: friendly ping to take a look at the changes/replies

(PS, I'm still trying to adjust to the GitHub review system - I'm not sure I'm holding it right - so if there's anything I should/shouldn't be doing for easy reviews please lmk).

domenic commented 4 years ago

Thanks for the ping! I'll do my best to get to this tomorrow.

bokand commented 4 years ago

Latest commit has all the latest feedback incorporated.

bokand commented 4 years ago

Thanks for the review and informative tips!

WICG / scroll-to-text-fragment

[Spec] Re-write text matching steps #90

:boom: Error: write EPROTO 139705468127104:error:1407742E:SSL routines:SSL23_GET_SERVER_HELLO:tlsv1 alert protocol version:../deps/openssl/openssl/ssl/s23_clnt.c:772: