1713301 - The response panel should show raw response when clicking on search results

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect, P2)

Description

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

STR:

1. Load http://softwareishard.com/tests/fission/case1/index.html
2. Open DevTools and select the Network panel
3. Reload the page, so there are some requests.
4. Open the search side panel and search for e.g. "button"
5. Click on one of the search results

AR:
The related request is properly selected and the side panel opened. The Response side panel is also properly selected, but not switched to "Raw" mode where the related line (search result location) is highlighted

ER:
The Response panel is auto switched to Raw mode

Honza

Comment 1

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

Sebastian could bug 1651649 caused this regression?
We have the same problem with the Request side panel.
I think it might be because of bug 1693147

Is this simple to fix?

Comment 2

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

:Honza, could you try to find a regression range using for example mozregression?

Comment 3

[Sebastian Zartner [:sebo]]

Hi Honza! Yes, that changes seem to have caused the regression. Sorry for that!

It might already be enough to change the state property rawDataDisplayed to true before highlighting the line. I didn't try it yet, though.

Sebastian

Comment 4

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

Sebastian, not sure if you have time for looking into this (we marked it as P2 since it's a regression). If not would you be available as a mentor for this?

Thank you,
Honza

Comment 5

[Sebastian Zartner [:sebo]]

I'll try to find some time this weekend to look into this. If I can't find a solution, I'd be happy to at least mentor somebody to fix this issue.

Sebastian

Comment 6

[Sebastian Zartner [:sebo]]

Attachment: Bug 1713301 - Fixed display of raw request or reponse data when clicking on search results. r=honza

Comment 7

[Sebastian Zartner [:sebo]]

Hi Honza! I've created a patch that should fix the issue. It doesn't include a test case yet, though I'll do so once you tell me whether the change works as expected.

Sebastian

Comment 8

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

Perfect thanks for looking into this!
Comments posted to Phab.

Honza

Comment 9

[Sebastian Zartner [:sebo]]

Honza, just in case you don't get notified automatically by the changes on Phab, I set the need-info flag again.

Sebastian

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d989c38eb4d9
Fixed display of raw request or reponse data when clicking on search results. r=Honza

Comment 11

[Noemi Erli[:noemi_erli]]

Backed out changeset d989c38eb4d9 (Bug 1713301) for causing failures in browser_net_search-results.js CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&selectedTaskRun=Feb8k4yHSZOdx3IfUAalEQ.1&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=d989c38eb4d951c668626f65afcbf6f2a0da8b7e

Log: https://treeherder.mozilla.org/logviewer?job_id=343466588&repo=autoland&lineNumber=8920

Backout: https://hg.mozilla.org/integration/autoland/rev/db7b183ad320f26f9f84b1d8004074b3e4657aa7

Comment 12

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

Sebastian, the patch got backed out since the: devtools/netmonitor/test/browser_net_search-results.js fails now
Here is a link to the original test failure
https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&selectedTaskRun=Feb8k4yHSZOdx3IfUAalEQ.1&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=d989c38eb4d951c668626f65afcbf6f2a0da8b7e

I pushed to try again and I am seeing the same, the test times out (I re-triggered the test, so it's still running atm, it's the suite dt9)
https://treeherder.mozilla.org/jobs?repo=try&revision=946a3324a7c37ad9e2d0af6de850a9388aab42d6&selectedTaskRun=LXFM1KmrQ9-5-k3r0FBW5Q.0

Here is a screenshot of Firefox made by treeherder when the test times out:
https://firefoxci.taskcluster-artifacts.net/LXFM1KmrQ9-5-k3r0FBW5Q/0/public/test_info/mozilla-test-fail-screenshot_f4_8sii6.png

Also, it seems to be only failing on Linux

Let me know if you want me to push another patch to Try to see the results.

Honza

Comment 13

[Sebastian Zartner [:sebo]]

I've now replaced the waitForDOMIfNeeded() call by one that's using waitFor(). Honza, could you please push it to Try?

Sebastian

Comment 14

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

Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c023c5f80f02727a6236aa0b1aa813597ab5008

Honza

Comment 15

[Sebastian Zartner [:sebo]]

@Honza need a Try push again, please! If it still doesn't work, I'd like to get some advice what's wrong because I cannot reproduce the problems locally that Treeherder faces. Thanks in advance!

Sebastian

Comment 16

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

Hi Sebastian,
sorry for the delay, Mozilla had week off last week.

Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5d20577823d4a7779804c277d5974e88c5bef8b2

Honza

Comment 17

[Sebastian Zartner [:sebo]]

Hi Honza,

no problem. Unfortunately, Try still reports some errors which I cannot reproduce. Could you give me some advice on what's wrong?

Sebastian

Comment 18

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

Attachment: image.png

I am attaching two screenshots:

• Win10 (at the top)
• Linux (at the bottom, from try)

The one from Win10 is showing 10 search results, which is what the test expects and what I am seeing when running it locally.
The one from Linux shows 12 (7 results for the second requests), which feels wrong.

Why is that happening?

Honza

Comment 19

[Sebastian Zartner [:sebo]]

It looks like on Linux the Cookie set by the first request is sent immediately with the second request, while on Windows it is not.
Regarding that, I think the output on Linux is actually the correct one. And I don't know why the cookie is not sent on Windows. It might be a configuration difference between both systems.

I'll try to come up with a different solution for the test, though this difference might also be a bug in the search panel code.

Sebastian

Comment 20

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

Yes, that sounds about right. I was testing locally on both Win and Linux and I think there is a race since both requests happens closely after each other. I am seeing sometimes that even on Linux the cookie isn't sent with the second request.

When executing the second request after a timeout (1sec) the results are consistent (on both Win and Linux): the cookie is sent with the second request.

I created a test page here:
http://janodvarko.cz/tests/bugzilla/1713301/

Perhaps we could execute the second request when the first is done in the test to make it stable?

I spot another small issue along the way and reported that as bug 1719266

Honza

Comment 21

[Sebastian Zartner [:sebo]]

Thank you for the detailed explanation! I've now changed the code so it waits for the requests to finish instead of performing them in parallel and I now get the full number of search results. So could you please do one last (hopefully) Try push for me?

During my tests I also saw an issue regarding the search result selections, so I created bug 1719369 for it now.

Sebastian

Comment 22

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

Of course, no problem, here is the push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=20f673c94691bbe71fd3eb8af4cbe812edba1d56

Honza

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e7e9d3e475b7
Fixed display of raw request or reponse data when clicking on search results. r=Honza

Comment 24

[Sebastian Zartner [:sebo]]

The race condition errors are obviously gone, though some accessibility issues are still listed in the last results. I went ahead and added the code for disabling the accessibility rules like it's done in browser_net_basic-search.js. With this change in place I am trying to land the changes again, now.

Sebastian

Comment 25

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/e7e9d3e475b7