Fix position for "Edit and Resend" and "Raw headers" buttons

RESOLVED FIXED in Firefox 55

Status

defect
P3
normal
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: magicp.jp, Assigned: tiago, Mentored)

Tracking

({good-first-bug})

unspecified
Firefox 56
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox53 unaffected, firefox54 wontfix, firefox55 fixed, firefox56 verified, firefox58 fixed)

Details

Attachments

(2 attachments)

Reporter

Description

2 years ago
[Steps to Reproduce]
1. Start Firefox 54 or later version
2. Open net monitor
3. Go to any site (e.g. https://www.mozilla.org/en-US/contribute/)
4. Reload current page for cache
5. Select any rows in status code 200 with green and gray icon

[Actual Results]
Position of "Edit and Resend" and "Raw headers" buttons is different by status.
"[Learn More]" is gone when status code 200 and gray icon. Is this intentional?

[Expected Results]
"Edit and Resend" and "Raw headers" buttons in the end side.
"[Learn More]" is shown even if status code 200 and gray icon.
Reporter

Updated

2 years ago
Mentor: odvarko
Keywords: good-first-bug
Priority: -- → P3
For anyone who is interested in fixing this bug:

1) The code responsible for rendering the [Learn More] link is here:
http://searchfox.org/mozilla-central/rev/cd8c561106d804e26bc09389f18f361846d005eb/devtools/client/netmonitor/src/components/headers-panel.js#200

2) It looks like `getHTTPStatusCodeURL` doesn't return valid URL (in the scenario described in comment #0)
http://searchfox.org/mozilla-central/rev/cd8c561106d804e26bc09389f18f361846d005eb/devtools/client/netmonitor/src/utils/mdn-utils.js#169

3) If there is no valid doc URL for the current status code (it might happen) the layout of the buttons should be still ok (aligned at the right)
Buttons are rendered here:
http://searchfox.org/mozilla-central/rev/cd8c561106d804e26bc09389f18f361846d005eb/devtools/client/netmonitor/src/components/headers-panel.js#203
They are using 'devetools-button' class. Perhaps we need more styling here...

Honza
Comment hidden (mozreview-request)
Assignee

Comment 3

2 years ago
Hi!

I just submitted a potential patch and I would like to ask for feedback on a couple things :)

On http://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/components/headers-panel.js#139

status has a string value for most cases I tried, in a green 200 has "200" and in a green 206 has "206".
The problem is on the grey 200, in that case, status has 200 (number) as a value. That is the cause of the [Learn More] link not showing for grey 200.

I couldn't find where that status value gets set, so for now, I went with a toString() solution. If someone could point me where the status is set, I'll go and try to fix the problem at its root.

As for the "Edit and Resend" and "Raw headers" buttons, these get pushed to the right by the [Learn More] link. 
In the scenario of this bug, the [Learn More] link is not there, so nothing pushes the buttons to the right.

My approach here was to add an empty span if there is no valid doc URL. This span has the same CSS responsible for pushing the buttons to the right.

I'm not sure if that's a good approach at all, maybe it's better to use an empty div instead, or giving the buttons the responsibility of being right aligned, not sure.

That's all, any feedback would be very appreciated!
Flags: needinfo?(odvarko)
Comment on attachment 8868234 [details]
Bug 1364153 - Fix position for 'Edit and Resend' and 'Raw headers' buttons.

https://reviewboard.mozilla.org/r/139810/#review143516

Looks reasonable, thanks for the patch!

R+

Honza
Attachment #8868234 - Flags: review?(odvarko) → review+
Flags: needinfo?(odvarko)

Updated

2 years ago
Assignee: nobody → tiago.paez11

Comment 5

2 years ago
Hi Honza, 
Did the code land ? or does it need 'checkin-needed' ? If it has landed, any idea why is it not marked as fixed yet ?
Flags: needinfo?(odvarko)

Comment 6

2 years ago
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a748ff0b5e48
Fix position for 'Edit and Resend' and 'Raw headers' buttons. r=Honza
(In reply to [:Towkir] Ahmed from comment #5)
> Hi Honza, 
> Did the code land ? or does it need 'checkin-needed' ? If it has landed, any
> idea why is it not marked as fixed yet ?
Ah, yes. If you want to have it landed you need te set 'checkin-needed' and one of the sheriffs will do it for you. I landed it myself now.

Thanks for the patch again!

Honza
Flags: needinfo?(odvarko)

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a748ff0b5e48
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
I have reproduced this bug with nightly 55.0a1 (2017-05-11) on "Linux Mint (64 Bit).

The bug's fix is now verified on Latest Nightly 56.0a1

Build ID 	20170701100236
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:56.0) Gecko/20100101 Firefox/56.0

[Bugday-20170628]

Comment 10

2 years ago
I have reproduced this Bug on Nightly 55.0a1 (2017-05-11) on Windows 10, 64 Bit!

The bug's fix is now verified on latest  Nightly 56.0a1

Build ID    :	20170701030203
User Agent  : 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0
QA Whiteboard: [bugday-20170628]
It's too late for 54. Mark 54 won't fix.

Hi :tiago, 
Do you think this is worth uplifting to Beta55?
Flags: needinfo?(tiago.paez11)
(In reply to Gerry Chang [:gchang] from comment #11)
> Hi :tiago, 
> Do you think this is worth uplifting to Beta55?

Same question for Honza.
Flags: needinfo?(odvarko)
Comment on attachment 8868234 [details]
Bug 1364153 - Fix position for 'Edit and Resend' and 'Raw headers' buttons.

Approval Request Comment
[Feature/Bug causing the regression]: -
[User impact if declined]: Wrong position (layout) of a button on the screen
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: no
[Why is the change risky/not risky?]: The feature is rather for power users and its only related to layout.
[String changes made/needed]: n/a
Flags: needinfo?(odvarko)
Attachment #8868234 - Flags: approval-mozilla-beta?
Comment on attachment 8868234 [details]
Bug 1364153 - Fix position for 'Edit and Resend' and 'Raw headers' buttons.

netmonitor fix, beta55+
Flags: needinfo?(tiago.paez11)
Attachment #8868234 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Jan Honza Odvarko [:Honza] from comment #13)
> [Is this code covered by automated tests?]: no
> [Has the fix been verified in Nightly?]: yes
> [Needs manual test from QE? If yes, steps to reproduce]: no

Setting qe-verify- based on Jan's assessment on manual testing needs.
Flags: qe-verify-
bugday-20171122

OS:Windows 10 (x64)
Browser:Firefox Beta 58
Status:Fixed and verified

I was able to reproduce the buy in the 54 build and the "Learn More" wasn't there on the on the status 200 Gray icon and the Position of "Edit and Resend" and "Raw headers" buttons is different by status.

Comment 18

2 years ago
[bugday-20171206]

OS:Linux debain 4.10.0-38-generic #42~16.04.1-Ubuntu x86_64 GNU/Linux
Distribution: elementary OS 0.4.1 Loki
Browser: Firefox Beta 58.0b3 (64-bit)
Status:Fixed and Verified

I was able to reproduce the bug in Firefox 56.0 (64-bit).
The bug has been fixed in Firefox Beta 58.0b3 (64-bit).

Updated

a year ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.