1731543 - Support formatters properly for ConsoleLogEntry for log.entryAdded event

Status: NEW

(Remote Protocol :: WebDriver BiDi, task, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Bug 1694143 implemented basic support for log.entryAdded by only handling ConcoleLogEntry event types partially. The remaining work still needs to be finished.

Julian, can you please tell us what exactly is needed here? Would be good to know for M2 point selection.

Comment 1

[Julian Descottes [:jdescottes]]

We still have to implement:

• support for arguments other than strings (might be blocked on Bug 1693839)
• support for realm (we always pass null at the moment)
• support for stacktrace (for which Bug 1731553 has been filed)

And then there is a question for serialized args (steps 6 & 7) when we have formatting arguments (%s ...)

According to the spec, we should apply formatting in step4. However in practice the formatting is already applied when we receive the message in the console-api-log-event (see exception in note 1).

I think we are supposed to forward the raw arguments in serialized args (steps 6 & 7). But we don't have access to the original arguments in case formatting was involved, because formatting was already applied. For instance if you log console.log("Hello %s!", "world") or console.log("Hello world!"), we get a console-api-log-event with a single Hello world! for both use cases.

So, if we really need to forward the raw arguments before formatting was applied, we will need to modify the Console.cpp implementation in order to forward additional information. See https://searchfox.org/mozilla-central/rev/3a8091d1c29473a0839ad7a5810028f41363fe2e/dom/console/Console.cpp#1794-1812. This is probably something worth checking with the specs before moving forward.

Note 1: There is one exception, with the %c formatters (CSS styling) which are not applied, because it's up to the "frontend" to decide how to apply those. This area is really under-specced https://console.spec.whatwg.org/#formatter.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Julian Descottes [:jdescottes] from comment #1)

We still have to implement:

• support for arguments other than strings (might be blocked on Bug 1693839)

This is not ConsoleLogEntry only related but applies to all types of entries. So support for that will be added independently from this bug.

• support for realm (we always pass null at the moment)

Ok, realm seems to be only used for this type of log entry.

• support for stacktrace (for which Bug 1731553 has been filed)

Right, nothing we have to track here.

And then there is a question for serialized args (steps 6 & 7) when we have formatting arguments (%s ...)

Ok, that also falls into this specific bug.

According to the spec, we should apply formatting in step4. However in practice the formatting is already applied when we receive the message in the console-api-log-event (see exception in note 1).

I think we are supposed to forward the raw arguments in serialized args (steps 6 & 7). But we don't have access to the original arguments in case formatting was involved, because formatting was already applied. For instance if you log console.log("Hello %s!", "world") or console.log("Hello world!"), we get a console-api-log-event with a single Hello world! for both use cases.

So, if we really need to forward the raw arguments before formatting was applied, we will need to modify the Console.cpp implementation in order to forward additional information. See https://searchfox.org/mozilla-central/rev/3a8091d1c29473a0839ad7a5810028f41363fe2e/dom/console/Console.cpp#1794-1812. This is probably something worth checking with the specs before moving forward.

Good for discussion in the triage meeting. In case we need platform support, we should get this filed sooner than later, and maybe have to work on that on our own.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

As discussed we should first check how Chrome behaves here, and if that would require us to update the BiDi specification.

Julian, maybe you could have look?

Comment 4

[Julian Descottes [:jdescottes]]

Using puppeteer's console event, I see the following for console.log("Hello %s", "world"):

• text is Hello %s world (no formatting applied)
• args is length 2, and contains the original string arguments Hello %s and world

So it seems that CDP is not applying any formatting at the moment, which they should do if they want to comply with the bidi spec. So maybe there can still be a discussion about this.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Interesting. So as it looks like there are two different ways console.log() can be called:
https://developer.mozilla.org/en-US/docs/Web/API/Console/log

Given what you have seen it looks like that Chrome only implements the first way, which appends the string representation of each passed object. It means that they might also have to do some platform work.

I had a look at the CDP documentation and it clearly contains an args entry for LogEntry: https://chromedevtools.github.io/devtools-protocol/tot/Log/#type-LogEntry. So passing over arguments is supported and as such we should do the same.

James, what would you propose? Shall we as best file a BiDi issue for discussion?

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

As discussed Julian will file an issue on BiDi repository to get this discussed.

Comment 7

[Julian Descottes [:jdescottes]]

Filed https://github.com/w3c/webdriver-bidi/issues/139 feel free to add clarifications if needed.

Comment 8

[Julian Descottes [:jdescottes]]

Might split it between realms & formatting. Formatting is blocked on discussions on the specs.

Comment 9

[Julian Descottes [:jdescottes]]

As discussed, I will review what happens when we use objects with formatters.

For a quick reference logging:

console.log(
`number: '%s',
string: '%s',
object: '%s',
object with toString '%s',
DOMElement: '%s'`, 1, "Some string", { a: 1 }, { toString :() => "Hi from toString" }, document.body)

Firefox DevTools:

number: '1',
string: 'Some string',
object: '[object Object]',
object with toString 'Hi from toString',
DOMElement: '[object HTMLBodyElement]'

Chrome DevTools:

number: '1',
string: 'Some string',
object: 'Object',
object with toString 'Object',
DOMElement: 'body'

Safari DevTools:

number: '1',
string: 'Some string',
object: 'Object',
object with toString 'Object',
DOMElement: '<body id="www-wikipedia-org">'

I will now look at what CDP returns

Comment 10

[Julian Descottes [:jdescottes]]

Repurposing to focus only on formatters. realms support is in Bug 1742589

Comment 11

[Julian Descottes [:jdescottes]]

ni: I should bring this topic back to the BiDi spec monthly meeting

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

We discussed internally 12 days ago: https://github.com/w3c/webdriver-bidi/issues/139#issuecomment-1097785708

Hi Julian, shall we seek for feedback from other vendors for the proposal of renaming the field or for the clarification that this field is vendor specific?

Comment 13

[Julian Descottes [:jdescottes]]

I will send a PR to gather feedback, haven't had time last week.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Julian Descottes [:jdescottes] from comment #13)

I will send a PR to gather feedback, haven't had time last week.

Btw the actual pull request is: https://github.com/w3c/webdriver-bidi/pull/200

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Note that this PR just got merged.

Julian could you briefly mention what needs to be done on this bug? Thanks.

Comment 16

[Julian Descottes [:jdescottes]]

Since there was no substantial modification to the spec, we need to do what was described in comment #1.

Right now formatting is applied before we get the event. For instance if you log console.log("Hello %s!", "world") we get a console-api-log-event with a single "Hello world!" argument, which makes it indistinguishable from a console.log("Hello world!").

We need to modify https://searchfox.org/mozilla-central/rev/3a8091d1c29473a0839ad7a5810028f41363fe2e/dom/console/Console.cpp#1794-1812 so that the original arguments can still be accessible for a console-api-log-event. We can still let Console.cpp perform the formatting and provide the "formatted string", as long as we still have access to the original arguments.

If we look at https://searchfox.org/mozilla-central/rev/3a8091d1c29473a0839ad7a5810028f41363fe2e/dom/console/Console.cpp#2048-2053, we can see that in case some formatting happened, the sequence of arguments is only made of the unused arguments. I don't imagine that we can change this to always return the original list arguments. We might have to introduce another sequence that will always hold the original arguments, regardless of what happened for formatting purposes.