Currently we have "Log Frame Text" and "Log Frame Value". The latter logs the frame as JSON, whether or not it actually is JSON. We should sniff the values prior to appending the menu items. If it is JSON, have "Log Frame as JSON" and "Log Frame as Text". If it's not JSON, then the option should be "Log Frame Value".
Since this uses RemoteObject to get the data, this change may require to sniff the content in the model side since we don't want to block showing the context menu on a round trip to get the remote object.
<rdar://problem/31487006>
(In reply to Brian Burg from comment #1) > Since this uses RemoteObject to get the data, this change may require to > sniff the content in the model side since we don't want to block showing the > context menu on a round trip to get the remote object. Are you suggesting to JSON.parse every message when it's added? If so, I'd have to test how much performance penalty it adds for big JSON messages.
(In reply to Nikita Vasilyev from comment #3) > (In reply to Brian Burg from comment #1) > > Since this uses RemoteObject to get the data, this change may require to > > sniff the content in the model side since we don't want to block showing the > > context menu on a round trip to get the remote object. > > Are you suggesting to JSON.parse every message when it's added? If so, I'd > have to test how much performance penalty it adds for big JSON messages. We already do this when the context menu is triggered. See the discussion and patches on bug 169945: https://bugs.webkit.org/show_bug.cgi?id=169945
(In reply to Brian Burg from comment #0) > Currently we have "Log Frame Text" and "Log Frame Value". The latter logs > the frame as JSON, whether or not it actually is JSON. I'm having trouble understanding this. We do know it is actually JSON, we've done a JSON.parse() on the contents to prove it. > We should sniff the values prior to appending the menu items. If it is > JSON, have "Log Frame as JSON" and "Log Frame as Text". If it's not > JSON, then the option should be "Log Frame Value". My expectations are: (1) if the frame is text => have a Log Text option (2) if the text is JSON => have a Log Value option which currently matches the code. Is there a specific case where the context menus were confusing?
(In reply to Joseph Pecoraro from comment #4) > (In reply to Nikita Vasilyev from comment #3) > > (In reply to Brian Burg from comment #1) > > > Since this uses RemoteObject to get the data, this change may require to > > > sniff the content in the model side since we don't want to block showing the > > > context menu on a round trip to get the remote object. > > > > Are you suggesting to JSON.parse every message when it's added? If so, I'd > > have to test how much performance penalty it adds for big JSON messages. > > We already do this when the context menu is triggered. > > See the discussion and patches on bug 169945: > https://bugs.webkit.org/show_bug.cgi?id=169945 Correct. We do this when we display the context menu, but not when a message is added.
> Correct. We do this when we display the context menu, but not when a message > is added. And this bug is talking about the context menu. I do not think we would want or need to do this when the message is added. Did you have a specific case in mind?
(In reply to Joseph Pecoraro from comment #7) > > Correct. We do this when we display the context menu, but not when a message > > is added. > > And this bug is talking about the context menu. I do not think we would want > or need to do this when the message is added. Did you have a specific case > in mind? No, I wanted to clarify what Brian meant in comment 1, that's all.
(In reply to Joseph Pecoraro from comment #5) > (In reply to Brian Burg from comment #0) > > Currently we have "Log Frame Text" and "Log Frame Value". The latter logs > > the frame as JSON, whether or not it actually is JSON. > > I'm having trouble understanding this. We do know it is actually JSON, we've > done a JSON.parse() on the contents to prove it. > > > We should sniff the values prior to appending the menu items. If it is > > JSON, have "Log Frame as JSON" and "Log Frame as Text". If it's not > > JSON, then the option should be "Log Frame Value". > > My expectations are: > > (1) if the frame is text => have a Log Text option > (2) if the text is JSON => have a Log Value option > > which currently matches the code. > > Is there a specific case where the context menus were confusing? "Value" is kinda confusing IMO since the text representation *is* the value. as JSON or as String would be clearer to me.