Log request and response bodies checkbox issues in Web Console

RESOLVED FIXED in Firefox 4.0b9

Status

P1
normal
RESOLVED FIXED
8 years ago
6 months ago

People

(Reporter: msucan, Assigned: msucan)

Tracking

Trunk
Firefox 4.0b9
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 final+)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

8 years ago
STR:

1. Open the Web Console.
2. Enable log request and response bodies (right-click).
3. Close the Web Console.
4. Reopen the Web Console.
5. Refresh the web page.
6. Click a network request log.

Expected result: the response body is not logged.

Actual result: the response body is logged. The menitem falsely shows the option  is disabled checkbox.

STR 2:

1. Open tabA and the Web Console inside it.
2. Open tabB and the Web Console inside it.
3. Enable logging of request and response bodies in tabA.
4. Refresh the Web Console inside tabB.
5. Click a network request log.

Expected result: the network response body is not logged.

Actual result: the network response body is logged.

Comment 1

8 years ago
I can see where this behavior comes from, but I agree that it's inconsistent and confusing. At the very least, the checkbox should show the true state of request/response body logging.

Also, if request/response body logging is tracked on a per site basis, we should turn it off when you close the console. Otherwise, people might have their memory consumed without knowing why (and the only way to stop it is to restart the browser or reopen the console on the right site and disable the logging...)
Assignee: nobody → mihai.sucan
Blocks: 594225

Comment 2

8 years ago
OK, looking at the patch in bug 598851, it appears that the logging is not tracked per site. So, we really just need to make sure that the check mark in the menu is always consistent.
(Assignee)

Comment 3

8 years ago
Created attachment 484652 [details] [diff] [review]
proposed patch

Proposed patch.
Attachment #484652 - Flags: feedback?(rcampbell)
(Assignee)

Updated

8 years ago
Status: NEW → ASSIGNED
Whiteboard: [patchclean:1020]
Comment on attachment 484652 [details] [diff] [review]
proposed patch

I don't see a corresponding menuPopup.removeEventListener in here. Do we do proper destruction of all these xul elements when we shutdown?
(Assignee)

Comment 5

8 years ago
(In reply to comment #4)
> Comment on attachment 484652 [details] [diff] [review]
> proposed patch
> 
> I don't see a corresponding menuPopup.removeEventListener in here. Do we do
> proper destruction of all these xul elements when we shutdown?

Yes, we do that in unregisterDisplay().

Comment 6

8 years ago
Requesting blocking for this bug. The "Log request and response bodies" checkbox in the context menu does not always show the true state of that logging, which would certainly be very confusing for the user.
blocking2.0: --- → ?
Blocking+ for broken UI.
blocking2.0: ? → final+
Comment on attachment 484652 [details] [diff] [review]
proposed patch

yup
Attachment #484652 - Flags: feedback?(rcampbell) → feedback+
(Assignee)

Comment 9

8 years ago
Created attachment 491202 [details] [diff] [review]
updated patch

Updated patch. Rebased and made small fixes needed.

Thanks Robert for the feedback+!
Attachment #484652 - Attachment is obsolete: true
Attachment #491202 - Flags: review?(dolske)
(Assignee)

Updated

8 years ago
Whiteboard: [patchclean:1020] → [patchclean:1117]

Comment 10

8 years ago
mass change: filter on PRIORITYSETTING
Blocks: 593956
Priority: -- → P1

Comment 11

8 years ago
mass change: filter mail on BLOCKERSETTING
Severity: normal → blocker
Severity: blocker → normal
Comment on attachment 491202 [details] [diff] [review]
updated patch

Normally this could be done with XUL observers, but I guess that's kind of awkward here since the DOM's constructed via JS.
Attachment #491202 - Flags: review?(dolske) → review+
(Assignee)

Comment 13

8 years ago
Created attachment 500599 [details] [diff] [review]
rebased patch

Rebased patch. Thanks for the review+!
Attachment #491202 - Attachment is obsolete: true
(Assignee)

Updated

8 years ago
Whiteboard: [patchclean:1117] → [patchclean:0101][checkin]
http://hg.mozilla.org/mozilla-central/rev/1494caa22150
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Whiteboard: [patchclean:0101][checkin]
Target Milestone: --- → Firefox 4.0b9

Updated

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