Closed Bug 1405245 Opened 3 years ago Closed 2 years ago

Migrate browser_webconsole_bug_1247459_violation.js to the new frontend


(DevTools :: Console, defect, P1)



(firefox60 fixed)

Firefox 60
Tracking Status
firefox60 --- fixed


(Reporter: nchevobbe, Assigned: jdescottes)


(Blocks 1 open bug)


(Whiteboard: [newconsole-mvp])


(2 files, 1 obsolete file)

No description provided.
Priority: -- → P3
Priority: P3 → P2
Whiteboard: [newconsole-mvp]
Assignee: nobody → jdescottes
Might have surfaced a small bug in PageError.js, regarding the handling of repeats.

Try with test migrated and enabled for new console:

Additional windows specific try run to see if we can enable the test on win e10s: 
(see Bug 1264955 ?)
Priority: P2 → P1
(xpcshell failures on my try run are linked to a mass breakage -> Bug 1435644)
We can land Bug 1428520 first and then land the test migration afterwards if needed.
Depends on: 1428520
Comment on attachment 8948388 [details]
Bug 1405245 - enable test-csp-violation.html for new webconsole;

::: devtools/client/webconsole/new-console-output/test/mochitest/head.js:125
(Diff revision 1)
> +              let repeatNode = newMessage.node.querySelector(".message-repeats");
> +              repeatMatch = repeatNode &&
> +                            (parseInt(repeatNode.textContent, 10) == repeat);

I think we could have a race here. 
We are waiting for a message to come, then check the repeat bubble right away.
But maybe the second similar message did not came in yet.
I think it could be better to wait until we do have a repeat node with the correct number (using waitFor maybe ?).

Also, I'm unsure about addind more options to waitForMessages. The old console had a lot of options to the point it was difficult to remember what was doing what. 
For our test, maybe we could split this in 2 : 1 call to waitForMessages, and another to a new waitForMessageRepeat for example, where we would wait for the repeat bubble.
Or, we could have waitForMessageRepeat wait for both the message and the repeat, but at least it is more explicit.

What do you think about this Julian ?
Comment on attachment 8948389 [details]
Bug 1405245 - enable test-csp-violation.html on windows e10s;

Nice fix ! :)
Attachment #8948389 - Flags: review?(nchevobbe) → review+
Attachment #8948389 - Attachment is obsolete: true
Comment on attachment 8948388 [details]
Bug 1405245 - enable test-csp-violation.html for new webconsole;

Great, thanks Julian
Attachment #8948388 - Flags: review?(nchevobbe) → review+
Comment on attachment 8948387 [details]
Bug 1405245 - webconsole PageError component read repeat from props;

Let's land this
Attachment #8948387 - Flags: review?(nchevobbe) → review+
Pushed by
webconsole PageError component read repeat from props;r=nchevobbe
enable test-csp-violation.html for new webconsole;r=nchevobbe
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Product: Firefox → DevTools
Duplicate of this bug: 1243978
Duplicate of this bug: 1428520
You need to log in before you can comment on or make changes to this bug.