search

CanisLupus / swift-selection-search

Swift Selection Search (SSS) is a simple Firefox add-on that lets you quickly search for some text in a page using your favorite search engines.
https://addons.mozilla.org/firefox/addon/swift-selection-search/
MIT License
215 stars 26 forks source link

Insert a space when concatenating text from adjacent block elements #164

Closed Smylers closed 4 years ago

Smylers commented 4 years ago

Describe the problem

If the selected text spans multiple lines in the browser, Swift Selection Search concatenates the text from each without a space between them, which can leading to the search text not reflecting what the user selected.

How to reproduce the problem

  1. Go to a Radio 2 track-listing.
  2. Select an artist name (which is in an <h3>) and a track name underneath it (in a <p>), like this: image
  3. See that if the user selects "The Script” on the first line and “Superheroes” on the next, these get joined together without a space, making the search term “The ScriptSuperheroes”, rather than the intended “The Script Superheroes”.

Presumably there aren't any space characters in the text nodes for those elements, so the search text is technically correct. But it isn't useful: to the user, a line-break (starting a new block) is clearly a word-break.

Obviously it'd be better not to insert spaces between inline elements (which could legitimately be spanning just part of a word). And maybe it isn't possible to handle cases where page authors have used CSS to repurpose default-iinline elements to be displayed as blocks. But hopefully it's possible to get cases like this right, where the elements <h3> and <p> are used appropriately.

Thanks.

System info:

sayashraaj commented 4 years ago

I want to contribute, pls assign

CanisLupus commented 4 years ago

@Smylers This makes sense and it's definitely something that should be fixed. ;)

I've always simply used the selection ranges given by window.getSelection(), which report selections like yours as a single range. Asking for the text in the range results in that concatenated version, so this needs another approach.

@sayashraaj Thank you for wanting to help! :) This would probably be fixed in page-script.ts before the line selection.text = selectedText; (possibly replacing the for-cycle right before it). I don't know much else about a possible solution at this point. Please let me know if you have questions.

Cheers! Daniel

sayashraaj commented 4 years ago

I have created a pull request. Please let me know if my attempt at the solution is correct or not, and I will try again if it is not correct. I am a beginner

CanisLupus commented 4 years ago

Hi @Smylers, version 3.41.0 should fix this. :)

Like I said to @sayashraaj here, the solution was actually to replace "new line" characters with spaces. Text selections like yours contain newlines and these were "undisplayable" in the single-line text field. They were also removed in the search since they are invalid in URLs, and so users never saw them at all.

I ended up replacing consecutive spaces/tabs/etc too to clean up some of these searches.

I'm closing this, but please let me know of any problems you find!