Closed Bug 1713301 Opened 6 months ago Closed 5 months ago

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

Categories

(DevTools :: Netmonitor, defect, P2)

defect

Tracking

(firefox-esr78 unaffected, firefox89 wontfix, firefox90 wontfix, firefox91 fixed)

RESOLVED FIXED
91 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox89 --- wontfix
firefox90 --- wontfix
firefox91 --- fixed

People

(Reporter: Honza, Assigned: sebo)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(2 files)

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

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?

Flags: needinfo?(sebastianzartner)
Has STR: --- → yes

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

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

Has Regression Range: --- → irrelevant
Flags: needinfo?(sebastianzartner)
Regressed by: 1651649, 1693147
Severity: -- → S3
Priority: -- → P2

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

Flags: needinfo?(sebastianzartner)

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

Flags: needinfo?(sebastianzartner)
Assignee: nobody → sebastianzartner
Status: NEW → ASSIGNED

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

Flags: needinfo?(odvarko)

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

Honza

Flags: needinfo?(odvarko)

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

Sebastian

Flags: needinfo?(odvarko)
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d989c38eb4d9
Fixed display of raw request or reponse data when clicking on search results. r=Honza

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

Flags: needinfo?(odvarko)

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

Sebastian

Flags: needinfo?(sebastianzartner) → needinfo?(odvarko)

@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

Flags: needinfo?(odvarko)

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

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

Honza

Flags: needinfo?(odvarko)

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

Flags: needinfo?(odvarko)
Attached image 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

Flags: needinfo?(odvarko)

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

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

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

Flags: needinfo?(odvarko)
Pushed by sebastianzartner@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e7e9d3e475b7
Fixed display of raw request or reponse data when clicking on search results. r=Honza

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

Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 91 Branch
You need to log in before you can comment on or make changes to this bug.