1719266 - Selecting a cookie search result doesn't highlight the right target

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect, P3)

Description

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

Attachment: image.png

When selecting a search result that represents a sent or received cookie the Cookies side panel is selected but both send and received cookie is selected in it.

STR:

1. Open DevTools select the Network panel
2. Load http://janodvarko.cz/tests/bugzilla/1713301/
3. Click the "Click Me!" button on the page
4. Open the Search side panel and search for test
5. Expand the second entry in the search results
6. select the first name1 test search results (there are two, one representing sent cookie and the other representing received cookie)
7. The Cookies side panel is properly selected but both cookies in it (received and sent) are highlighted

ER: just the one proper cookie should be selected in the Cookies side panel

Honza

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

:Honza, if you think that's a regression, could you try to find a regression range using for example mozregression?

Comment 2

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

I am unsure whether this is a regression, but it's worth checking out.

Comment 3

[Alexandru Trif, Desktop Test Engineering [:atrif]]

Hello! Attaching regression range made on Windows 10x64. Hope it helps.

Last good revision: 99066bbd1b3a288471f2bf7f59f1c99da42610a1 (2020-02-11)
First bad revision: e84a0546e6e2e1f7a7b5912c86e3e3cab311555c (2020-02-12)
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=99066bbd1b3a288471f2bf7f59f1c99da42610a1&tochange=e84a0546e6e2e1f7a7b5912c86e3e3cab311555c

Comment 4

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

Perfect, thank you Alexandru!

I believe that this bug is causing the regression:
Bug 1613881 - Use Accordion for the Cookies Panel

Comment 5

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

Some instructions for anyone interested in fixing this bug:

1. The Cookies panel is rendered using CookiesPanel component
   https://searchfox.org/mozilla-central/rev/9c9f2f85d00aef1943cb521ac95c72bae932ebc5/devtools/client/netmonitor/src/components/request-details/CookiesPanel.js#42-46

2. Request and Response cookies lists are rendered using PropertiesView component (each list using its own instance)
   https://searchfox.org/mozilla-central/rev/9c9f2f85d00aef1943cb521ac95c72bae932ebc5/devtools/client/netmonitor/src/components/request-details/CookiesPanel.js#122-150

Note that the panel passes targetSearchResult object into the PropertiesView component as a property

3. The PropertiesView is selecting its items using this logic
   https://searchfox.org/mozilla-central/rev/9c9f2f85d00aef1943cb521ac95c72bae932ebc5/devtools/client/netmonitor/src/components/request-details/PropertiesView.js#221-224

• Either using the targetSearchResult prop and helper getSelectedPath method (which is the case in the Cookies panel)
• Or using custom selectPath function passed in as prop

The default getSelectedPath is too simple and just returns targetSearchResult.label, which is the same for both sent/received cookie - thus both are selected -> BUG

We need to pass a custom selectPath function in the PropertiesView

4. Luckily the Headers panel does the same
   https://searchfox.org/mozilla-central/rev/9c9f2f85d00aef1943cb521ac95c72bae932ebc5/devtools/client/netmonitor/src/components/request-details/HeadersPanel.js#286-291

It implements a function that returns proper path that is used to generate tree-identifier (a path) utilized by the PropertiesView to know, which item should be selected. Note that headers are also rendered using the PropertiesView component.

The function is passed into the PropertiesView

• for response headers https://searchfox.org/mozilla-central/rev/9c9f2f85d00aef1943cb521ac95c72bae932ebc5/devtools/client/netmonitor/src/components/request-details/HeadersPanel.js#568
• for request headers https://searchfox.org/mozilla-central/rev/9c9f2f85d00aef1943cb521ac95c72bae932ebc5/devtools/client/netmonitor/src/components/request-details/HeadersPanel.js#599

The goal is to implement similar function for the Cookies panel (or share it with the HeadersPanel component?) and to the same for cookies

Comment 6

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

Muhammadali, are you interested in working on this bug?

Honza

Comment 7

[Mustafa]

Hi Honza,

I am interested to work on this bug.
is it possible to assign it to me?

Mustafa

Comment 8

[Mustafa]

Attachment: WIP: Bug 1719266 - highlight the right selected cookie. :Honza

Comment 9

[Mustafa]

Attachment: Bug 1719266 - [devtools] Highlight the right selected cookie. :Honza

remove left-padding in cookiesPanel

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4051d2792ad2
[devtools] Highlight the right selected cookie. :Honza r=Honza DONTBUILD

Comment 11

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/4051d2792ad2

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Is this something we should consider uplifting to Beta?

Comment 13

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

No need to rush this, let's bake it in Nightly.
Thanks for asking though.
Honza