?

Success

User tests: Successful: Unsuccessful:

avatar Fedik
Fedik
15 Oct 2015

this fix for #8034
the problem was that $ is striped and the highlighter script tries to highlight the emptiness, that cause high resource usage by highlight.js

for testing please look the description in related issue :wink:


Steps to reproduce the issue

  • enable smart search & smart search module
  • index content so it's not empty
  • search for "$" (single dollar sign)

Expected result

some search results

Actual result

javascript loop resulting in massive memory leak and eventual browser crash

System information (as much as possible)

browser = firefox
OS = Win 10

Additional comments

avatar Fedik Fedik - open - 15 Oct 2015
avatar Fedik Fedik - change - 15 Oct 2015
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 15 Oct 2015
Labels Added: ?
avatar Fedik Fedik - change - 15 Oct 2015
Title
Fix for #8034 - do nothing when nothing to highlight
Fix for #8034 - smart search fail on $
avatar alikon
alikon - comment - 15 Oct 2015

tested succesfully now no more loop and browser crash
not sure if is it out of scope of this pr but
something is still wrong :

wrong text $ sign is missed
smart search

wrong search result the $$$ is not matched/highlight
smart search

avatar Fedik
Fedik - comment - 15 Oct 2015

the problem was that $ is striped and the highlighter tried highlight emptiness, that cause high resource usage by highlight.js

avatar alikon
alikon - comment - 15 Oct 2015

i'm a js :baby_chick:, just noticed that for search works as expected
search

surely @dgt41 can help us

avatar Fedik
Fedik - comment - 15 Oct 2015

$ is striped not in javascript, but somewhere in Smart Search component while handle the input ... I have not searched :smile:

avatar chrisdavenport
chrisdavenport - comment - 15 Oct 2015

I haven't checked the code, but I would think that all "punctuation" characters will be stripped. Same for the indexer. That would be expected behaviour.

avatar zero-24 zero-24 - change - 15 Oct 2015
Category Search
avatar alikon
alikon - comment - 16 Oct 2015

@chrisdavenport Not so sure we can consider $ , € etc ..... a punctuation character
or at least not in all languages
so i think com_search is doing the correct behavior
what i'm missing?

avatar chrisdavenport
chrisdavenport - comment - 16 Oct 2015

For the kind of websites I normally work with I wouldn't want these characters cluttering up the index. However, I can imagine that there would be occasions where some websites might want to retain them. I would suggest that this needs to be configurable because it depends on the particular application. It's rather like how we currently have a list of "stop words" that are not indexed. I think we should have a configurable list of "stop characters" that are discarded and/or regarded as delimiters. I'd be happy to review a PR for that feature.

avatar alikon
alikon - comment - 21 Oct 2015

:+1:

p.s.
i'm sorry for my shortsight, i usually work with ($, €) characters :alien:
damned banking software work

avatar chrisdavenport chrisdavenport - test_item - 8 May 2016 - Tested successfully
avatar chrisdavenport
chrisdavenport - comment - 8 May 2016

I have tested this item :white_check_mark: successfully on f3f7e62

The specific issue reported by @Fedik is fixed by this PR.

The consequential issue reported by @alikon should be the subject of a separate PR along the lines I suggested.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8097.

avatar zero-24 zero-24 - test_item - 8 May 2016 - Tested successfully
avatar zero-24
zero-24 - comment - 8 May 2016

I have tested this item :white_check_mark: successfully on f3f7e62

This specific issue is fixed. Thanks.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8097.

avatar zero-24 zero-24 - change - 8 May 2016
Status Pending Ready to Commit
avatar zero-24
zero-24 - comment - 8 May 2016

RTC. Thanks


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8097.

avatar joomla-cms-bot joomla-cms-bot - change - 8 May 2016
Labels Added: ?
avatar brianteeman brianteeman - change - 8 May 2016
Milestone Added:
avatar Kubik-Rubik
Kubik-Rubik - comment - 8 May 2016

Thank you @Fedik and testers!

avatar Kubik-Rubik Kubik-Rubik - change - 8 May 2016
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2016-05-08 13:46:42
Closed_By Kubik-Rubik
avatar Kubik-Rubik Kubik-Rubik - close - 8 May 2016
avatar Kubik-Rubik Kubik-Rubik - merge - 8 May 2016
avatar joomla-cms-bot joomla-cms-bot - close - 8 May 2016
avatar joomla-cms-bot joomla-cms-bot - change - 8 May 2016
Labels Removed: ?

Add a Comment

Login with GitHub to post a comment