1668985 - Incorrect order of message columns in Response Panel

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect, P2)

Description

[Farooq AR]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:81.0) Gecko/20100101 Firefox/81.0

Steps to reproduce:

1. Go to https://www.websocket.org/echo.html
2. Click "Connect"
3. Send one or more messages by clicking "Send"
4. Open Network Monitor
5. Select the websocket request
6. Open Response tab in side panel
7. Right click on the messages table header and select "MaskBit" option

Actual results:

The "MaskBit" column shows the time and the "Time" column shows the maskbits.

Expected results:

The "MaskBit" column should show maskbits and the "Time" column should show time.

Comment 1

[Farooq AR]

The culprit is likely to be the call to Object.entries at MessageListContent.js#261 which results in a different order than the code at MessageListHeader.js#58

Comment 2

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

:farooqbckk, if you think that's a regression, then could you try to find a regression range in using for example mozregression?

Comment 3

[Peter Magyari (Desktop QA)]

I can also reproduce this issue, what I observed was that this also occurs when adding OpCode or FinBit column.

I've ran a mozregression and it pointed to Bug 1637867

Comment 4

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

@Farooq @Peter: thanks for the report, testing and regression window!

(In reply to Farooq AR from comment #1)

The culprit is likely to be the call to Object.entries at MessageListContent.js#261 which results in a different order than the code at MessageListHeader.js#58

The following change fixes the problem for me:

diff --git a/devtools/client/netmonitor/src/components/messages/MessageListContent.js b/devtools/client/netmonitor/src/components/messa--- a/devtools/client/netmonitor/src/components/messages/MessageListContent.js
+++ b/devtools/client/netmonitor/src/components/messages/MessageListContent.js
@@ -24,17 +24,19 @@ const { table, tbody, tr, td, div, input
 const { L10N } = require("devtools/client/netmonitor/src/utils/l10n");
 const MESSAGES_EMPTY_TEXT = L10N.getStr("messagesEmptyText");
 const TOGGLE_MESSAGES_TRUNCATION = L10N.getStr("toggleMessagesTruncation");
 const TOGGLE_MESSAGES_TRUNCATION_TITLE = L10N.getStr(
   "toggleMessagesTruncation.title"
 );
 const CONNECTION_CLOSED_TEXT = L10N.getStr("netmonitor.ws.connection.closed");
 const Actions = require("devtools/client/netmonitor/src/actions/index");
-
+const {
+  MESSAGE_HEADERS,
+} = require("devtools/client/netmonitor/src/constants.js");
 const {
   getSelectedMessage,
 } = require("devtools/client/netmonitor/src/selectors/index");

 // Components
 const MessageListContextMenu = require("devtools/client/netmonitor/src/components/messages/MessageListContextMenu");
 loader.lazyGetter(this, "MessageListHeader", function() {
   return createFactory(
@@ -253,19 +255,19 @@ class MessageListContent extends Compone

     if (messages.length === 0) {
       return div(
         { className: "empty-notice message-list-empty-notice" },
         MESSAGES_EMPTY_TEXT
       );
     }

-    const visibleColumns = Object.entries(columns)
-      .filter(([name, isVisible]) => isVisible)
-      .map(([name]) => name);
+    const visibleColumns = MESSAGE_HEADERS
+      .filter(header => columns[header.name])
+      .map(col => col.name);

     let displayedMessages;
     let MESSAGES_TRUNCATED;
     const shouldTruncate = messages.length > this.messagesLimit;
     if (shouldTruncate) {
       // If the checkbox is checked, we display all messages after the currentedTruncatedNum limit.
       // If the checkbox is unchecked, we display all messages after the messagesLimit.
       this.currentTruncatedNum = this.state.checked

Farooq, can you please test this and let me know if it works for you?

Also, do you think we could modify the new browser_net_ws-sse-persist-columns.js test and check also values in the columns?

This is annoying bug and it would be great to fix that ASAP.

Thanks!

Honza

Comment 5

[Farooq AR]

Thanks Honza for the diff, it works.

Comment 6

[Farooq AR]

Attachment: Bug 1668985 - Incorrect order of message columns in Response Panel. r=honza

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2f382e5f7e7a
Incorrect order of message columns in Response Panel. r=Honza

Comment 8

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/2f382e5f7e7a

Comment 9

[Peter Magyari (Desktop QA)]

Verified fixed on Firefox Nightly 83.0a1 (20201008210150)

Comment 10

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

Comment on attachment 9180355 [details]
Bug 1668985 - Incorrect order of message columns in Response Panel. r=honza

Beta/Release Uplift Approval Request

• User impact if declined: Data incorrectly displayed in columns for WebSocket SeverSideEvents traffic.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Only affects Web Developers inspecting WS and/or SSE. Rather smaller patch.
• String changes made/needed:

Comment 11

[Julien Cristau [:jcristau]]

This is S3 and we're about to build the 82 release candidate, I think this should ride to 83.