862851 - Please swap the Domain and File columns in the network panel

Status: VERIFIED FIXED

(DevTools :: Netmonitor, defect, P3)

Description

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

I find myself looking at the wrong place. Wouldn't it make sense to show domain first like it appears in URLs anyway?

Comment 1

[Victor Porof [:vporof][:vp]]

Shorlander, what do you think about this? I tend to agree.

Comment 2

[Stephen Horlander [:shorlander]]

It is more consistent with the way we typically read URLs. Although on the other hand the file is probably more relevant than the domain in a lot of cases.

I would flip them for now. I think we talked about potentially making them arrangeable/hiddeable in the future?

Comment 3

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

I think we should be consistent with Chrome DevTools in this case.
I.e. not swap and hide the domain column by default

Honza

Comment 4

[Ariel Burone]

Attachment: bug862851.patch

First patch hidding the Domain column by default

Comment 5

[Ariel Burone]

Sorry. Ignore the patch (can't delete it) at first looked like it worked but later not :(

Comment 6

[Ariel Burone]

Attachment: bug862851.patch

Fixed.
The first patch worked but upon selecting "reset columns", domain would reappear.
ui.js stores the defaults.

I tried to reorder the columns to provide a 2nd patch emulating chrome distribution but couldn't do it. I suppose there's a 4th file setting the order that I can't find?

Comment 7

[Ariel Burone]

Attachment: Summary: Hides Domain column from network panel by default

Comment 8

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

@Ariel, thanks for helping with this one!

Sorry for the confusion here. It looks like swapping the Domain and File columns is the right way to go in this report.

Can you please update your patch accordingly?

Honza

Comment 9

[Ariel Burone]

Attachment: Sumary: Bug 862851 - Swap the Domain and File columns in the network panel

The only file that really needs to be changed is constants.js. But
decided to change the order in the other files for consistency sake.

Comment 10

[Ariel Burone]

Comment on attachment 9018814 [details]
Summary: Hides Domain column from network panel by default

Done

The only file that really needed to be changed is constants.js. But
decided to change the order in the other files for consistency sake.

Comment 11

[Ariel Burone]

Thanks for the patience.
I can't mark the older files as obsolete, I suppose I don't have permissions to do so.

Comment 12

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

Attachment: image.png

The column-header-titles are swapped, but not the column content.
Honza

Comment 13

[Ariel Burone]

(In reply to Jan Honza Odvarko [:Honza] need-info me from comment #12)
> Created attachment 9020732 [details]
> image.png
> 
> The column-header-titles are swapped, but not the column content.
> Honza

Sorry, I was distracted.

Here's the fix
https://phabricator.services.mozilla.com/D9989

Comment 14

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

(In reply to Ariel Burone from comment #13)
> Here's the fix
> https://phabricator.services.mozilla.com/D9989

Great, thanks!

Couple of comments:

1) The patch looks good I am waiting for Try results
https://treeherder.mozilla.org/#/jobs?repo=try&revision=421c05334e0b7ac56e5307c569ddb5f78b234430
We can land if tests are green

2) Please mark all non-relevant or obsolete patches as obsolete (Attachment -> Details -> Edit Details -> Check Obsolete checkbox)

3) The patch in Phabricator doesn't have reviewer set. Also there is no need-info here. That means that you request for review is not in my queue and can be simply to be lost in the bug-mail.


Honza

Comment 15

[Ariel Burone]

(In reply to Jan Honza Odvarko [:Honza] (need-info? me) from comment #14)
> (In reply to Ariel Burone from comment #13)
> > Here's the fix
> > https://phabricator.services.mozilla.com/D9989
> 
> Great, thanks!
> 
> Couple of comments:
> 
> 1) The patch looks good I am waiting for Try results
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=421c05334e0b7ac56e5307c569ddb5f78b234430
> We can land if tests are green
> 
> 2) Please mark all non-relevant or obsolete patches as obsolete (Attachment
> -> Details -> Edit Details -> Check Obsolete checkbox)

I can't :( don't have permissions to do so.
The only moment I cant mark as obsolete is when I upload am attachment.

> 
> 3) The patch in Phabricator doesn't have reviewer set. Also there is no
> need-info here. That means that you request for review is not in my queue
> and can be simply to be lost in the bug-mail.

Just found how to do so from phabricator. Thanks for the heads up!

> 
> 
> Honza

When I reply to a message like this. Do you see something different? or it's just another normal looking email?
Thanks again,
Ariel

Comment 16

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

(In reply to Ariel Burone from comment #15)
> I can't :( don't have permissions to do so.
> The only moment I cant mark as obsolete is when I upload am attachment.
I marked the attached patch as obsolete.
Can you remove the on Phabricator revision?

> > 3) The patch in Phabricator doesn't have reviewer set. Also there is no
> > need-info here. That means that you request for review is not in my queue
> > and can be simply to be lost in the bug-mail.
> 
> Just found how to do so from phabricator. Thanks for the heads up!
Excellent!


> When I reply to a message like this. Do you see something different? or it's
> just another normal looking email?
Not sure what you mean, bug my bug-mail (for this comment) looks the same as others...

New try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=07b23dcd81ef67db553f5561866a1b4d5cfdbce9
(the previous was wrong)

Honza

Comment 17

[Ariel Burone]

> Not sure what you mean, bug my bug-mail (for this comment) looks the same as
> others...


Ok. Yes that's was I was asking. 
Thanks

About the obsoletes. The only moment I can mark as obsolete is if I attach a new file from Bugzilla. I couldn't find another way to do so. 


Odd thing:
Every time I used arc diff, it automatically associated the upload with the bug in bugzilla, but not this time, and I've got no errors at all.

Comment 18

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

> New try push:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=07b23dcd81ef67db553f5561866a1b4d5cfdbce9
Green

Honza

(In reply to Ariel Burone from comment #17)
> Every time I used arc diff, it automatically associated the upload with the
> bug in bugzilla, but not this time, and I've got no errors at all.

In order to update a patch in Phabricator you should use --update

Example:

# upload the last commit
arc diff .^ --update D3225

Honza

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c975dd62f9d3
Sumary: Bug 862851 - Swap the Domain and File columns in the network panel r=Honza

Comment 20

[Andrei Ciure[:aciure]]

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

Comment 21

[Daniel Cicas [:dcicas], Release QA]

Hello,

I reproduced this issue using Fx 65.0a1, build ID: 20181027100344, on MacOS 10.13.

I can confirm this issue is verified as fixed using Fx 65.0b5, build ID:  20181217180946, on Windows 10 x32, macOS X 10.13 and Ubuntu 18.04.