1500391 - [remote-dbg-next] Implement serviceworker configuration warning and migrate browser_service_workers_not_compatible.js

Status: RESOLVED FIXED

(DevTools :: about:debugging, enhancement, P1)

Description

[Julian Descottes [:jdescottes]]

Migrate https://searchfox.org/mozilla-central/source/devtools/client/aboutdebugging/test/browser_service_workers_not_compatible.js to the new about:debugging

Comment 1

[Julian Descottes [:jdescottes]]

filter on remote-debugging-next-move-m3-to-m2

Comment 2

[Julian Descottes [:jdescottes]]

filter on remote-debugging-next-move-m3-to-m2

Comment 3

[Julian Descottes [:jdescottes]]

filter on remote-debugging-next-move-m3-to-m2

Comment 4

[Julian Descottes [:jdescottes]]

The feature is currently not implemented in the new about:debugging so we should also address the gap at the same time.

Comment 5

[Julian Descottes [:jdescottes]]

Probably best to wait until Bug 1488502 lands to avoid too many conflicts. Both bugs revolve around the same areas.

Comment 7

[Julian Descottes [:jdescottes]]

You can have a look at the following documentation for some tips on how to migrate tests: https://gist.github.com/juliandescottes/fecc426ac84600357259bf513cea744c

Comment 8

[Julian Descottes [:jdescottes]]

Attachment: warning+link.png

Matt, do you know if it's tolerated to have learn more links in Warning messages? We currently do that in about:debugging and I am porting the feature to the new about:debugging. The learn more link goes to MDN.

Comment 9

[Julian Descottes [:jdescottes]]

Attachment: Bug 1500391 - Show warning message in runtime if service workers are unavailable;r=ladybenko

Comment 10

[Julian Descottes [:jdescottes]]

Attachment: Bug 1500391 - Migrate test for service workers not compatible;r=ladybenko

Depends on D17401

Comment 11

[mcroud]

Photon docs say that a good warning message should:

"Include information that will help users prevent an immediate issue and avoid similar issues in the future."

Also the message bar Doc states that the message body should:

"...Be descriptive and include any troubleshooting actions"

To me, providing a Learn more link within the message upholds these requirements.

The docs also provide a visual example of a warning message with an actionable button:
https://firefox-dev.tools/photon/components/message-bars.html#warning
Unless this message will also contain a button, perhaps a Learn more button might appease the Photon docs more?

Comment 12

[Julian Descottes [:jdescottes]]

Thanks for the feedback Matt! I have tried using a button and I think it might be worth keeping this for a follow up because it brings a few additional questions:

1. the photon docs suggest different background colors for the buttons in the warning message on :hover and :active:

   • hover is Yellow 70 #a47f00
   • active is Yellow 80 #715100
     Knowing that the text color for warning messages is Yellow 90, the contrast is extremely bad for those two states. Should we use a different text color?

2. What should happen when the message is too long to fit the width of the container? should we force it to be on one line and add an ellipsis, should we go multiline and display the button centered next to it, multiline but button aligned top ?

+--------------------------------------+
| Test is too long but... [learn more] |
+--------------------------------------+

+--------------------------------------+
| Test is too long but we              | 
| can put it on several   [learn more] |
| lines so this is ok.                 | 
+--------------------------------------+

+--------------------------------------+
| Test is too long but we [learn more] | 
| can put it on several                |
| lines so this is ok.                 | 
+--------------------------------------+

There is a mention about this in https://firefox-dev.tools/photon/components/message-bars.html#warning (last item of the page). They suggest to break the button into a new line when the message is on multiple lines, but in my situation I don't know if the message will be on multiple lines (depends on localization + viewport width). Should I then assume multiline?

+--------------------------------------+
| Even if the text is short...         |
| [learn more]                         |
+--------------------------------------+

Comment 13

[mcroud]

Attachment: warning-message-contrast.png

Your right! I've created an image showing the hover/pressed states and they really struggle and fail accessible contrast. Interestingly changing to white only marginally bumps up the contrast ratio but makes so much difference to me personally. Let me post this issue (and the attached) on the Photon repo to see what guidance I can get.

Yes this section in the docs could cover more scenarios regarding variable width and button positioning. I can't provide the correct answer without further enquiries but assuming mutliline will occur and having the button at the bottom as per the "Message on Multiple Lines" image in the docs means we've followed what is written.

If it looks a bit odd when there's a small message then I have something to add that the docs should cover in the future.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/357d8c184220
Show warning message in runtime if service workers are unavailable;r=ladybenko
https://hg.mozilla.org/integration/autoland/rev/3dafc8f144ed
Migrate test for service workers not compatible;r=ladybenko

Comment 15

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/357d8c184220
https://hg.mozilla.org/mozilla-central/rev/3dafc8f144ed