1364386 - Search in frame function shouldn't check for the host

Status: VERIFIED FIXED

(DevTools :: Console, defect, P1)

Description

[Nicolas Chevobbe [:nchevobbe]]

STR: 
1. Go to https://bgrins.github.io/devtools-demos/console/stress.html
2. Click the stream button and again to stop the stream
3. Open the console
4. In the filter field, enter "htt"

Expected result:
All the messages should be hidden

Actual result:
The messages are still visible

---

This is because we check for a match in the frame of a message, which contains the whole URL https://bgrins.github.io/devtools-demos/console/stress.html.

But in the UI, the Frame component only show "stress.html".
So maybe we should run the getSourceNames function (http://searchfox.org/mozilla-central/source/devtools/client/shared/source-utils.js#112) and compare to the short name.

The old console was checking only what was shown on screen and we should do the same.

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: Bug 1364386 - Use only file name for search;

Review commit: https://reviewboard.mozilla.org/r/139756/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/139756/

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8868148 [details]
Bug 1364386 - Use only file name for search;

https://reviewboard.mozilla.org/r/139756/#review143122

Thanks Honza !
Could you add a mochitest to make sure we don't pick the whole file path in search anymore ?

::: devtools/client/webconsole/new-console-output/selectors/messages.js:137
(Diff revision 1)
> +  return [short, frame.line, frame.column]
>      .join(":")

nit: since we're explicitely picking the things that we need now, I wonder if it wouldn't be faster to directly use a string

```
return `${short}:{frame.line}:{frame.column}`
  .toLocaleLowerCase()
  .includes(text.toLocaleLowerCase());
```

so we don't have to create an array and join it.

Feel free to keep it like it is, I'm not feeling strong about this

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8868148 [details]
Bug 1364386 - Use only file name for search;

https://reviewboard.mozilla.org/r/139756/#review143122

I meant a mocha test

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: Bug 1364386 - Update tests;

Review commit: https://reviewboard.mozilla.org/r/140154/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/140154/

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8868148 [details]
Bug 1364386 - Use only file name for search;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139756/diff/1-2/

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #2)
> Comment on attachment 8868148 [details]
> Bug 1364386 - Use only file name for search;
> 
> https://reviewboard.mozilla.org/r/139756/#review143122
> 
> Thanks Honza !
> Could you add a mochitest to make sure we don't pick the whole file path in
> search anymore ?
Done 

> ::: devtools/client/webconsole/new-console-output/selectors/messages.js:137
> (Diff revision 1)
> > +  return [short, frame.line, frame.column]
> >      .join(":")
> 
> nit: since we're explicitely picking the things that we need now, I wonder
> if it wouldn't be faster to directly use a string
Done

Honza

Comment 7

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8868148 [details]
Bug 1364386 - Use only file name for search;

https://reviewboard.mozilla.org/r/139756/#review143778

Looks good !

Comment 8

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8868539 [details]
Bug 1364386 - Update tests;

https://reviewboard.mozilla.org/r/140154/#review143780

Thanks Honza, looking all good

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/75bc9f142325
Use only file name for search; r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/b02281ac790d
Update tests; r=nchevobbe

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/75bc9f142325
https://hg.mozilla.org/mozilla-central/rev/b02281ac790d

Comment 11

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

Managed to reproduce the initial issue on 55.0a1 (2017-04-23). I can confirm the bug is verified fixed on 55.0a1 (2017-05-21), using Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.11.6.