1620932 - Fix React warning in devtools/client/responsive/components/Browser.js `allowFullScreen: "true"`

Status: RESOLVED FIXED

(DevTools :: Responsive Design Mode, task, P3)

Description

[Julian Descottes [:jdescottes]]

What were you doing?

1. Build Firefox with the build option ac_add_options --enable-debug-js-modules
2. Run Firefox
3. Open RDM

What happened?

The following warning is logged in the command line output:

console.error: "Warning: Received the string `%s` for the boolean attribute `%s`. %s 
Did you mean %s={%s}?%s" "true" "allowFullScreen" 
"Although this works, it will not work as expected if you pass the string \"false\"." 
"allowFullScreen" "true"
in iframe (created by Browser)
in Browser (created by ResizableViewport)
in div (created by ResizableViewport)
in div (created by ResizableViewport)
in div (created by ResizableViewport)
in ResizableViewport (created by Viewports)
in div (created by Viewports)
in div (created by Viewports)
in Viewports (created by Connect(Viewports))
in Connect(Viewports) (created by App)
in div (created by App)
in App (created by Connect(App))
in Connect(App)
in Provider"

What should have happened?

No error should be logged.

Anything else we should know?

Not sure why the error message is so unreadable, but the fix should be easy:
https://searchfox.org/mozilla-central/rev/557a0e222dd104c5d805ba344c45d6abc27d3db0/devtools/client/responsive/components/Browser.js#163

allowFullScreen: "true",

should be changed to

allowFullScreen: true,

This should be a good-first-bug for someone who wants to setup their development environment.
Out contributors documentation is at https://docs.firefox-dev.tools/

Comment 1

[Maria Ortiz [ :marylicious ]]

I want to work on this !

Comment 2

[Julian Descottes [:jdescottes]]

Hi marylicious!

Thanks for offering to help with this, I'm assigning the bug to you.
Take a look at https://docs.firefox-dev.tools/ to get started, and don't hesitate to ask for help here or on slack/riot if needed.

Comment 3

[Maria Ortiz [ :marylicious ]]

I'm following the steps to reproduce but the console doesn't log this error. Just these ones:

console.error: "Warning: Failed prop type: Toolbar: prop type `devices.phones[0].os` is invalid; it must be a function, usually from the `prop-types` package, but received `undefined`.\n    in Toolbar (created by Connect(Toolbar))\n    in Connect(Toolbar) (created by App)\n    in div (created by App)\n    in App (created by Connect(App))\n    in Connect(App)\n    in Provider"
console.error: "Warning: Failed prop type: DeviceSelector: prop type `devices.phones[0].os` is invalid; it must be a function, usually from the `prop-types` package, but received `undefined`.\n    in DeviceSelector (created by Toolbar)\n    in Toolbar (created by Connect(Toolbar))\n    in Connect(Toolbar) (created by App)\n    in div (created by App)\n    in App (created by Connect(App))\n    in Connect(App)\n    in Provider"
console.error: "Warning: Failed prop type: DevicePixelRatioMenu: prop type `devices.phones[0].os` is invalid; it must be a function, usually from the `prop-types` package, but received `undefined`.\n    in DevicePixelRatioMenu (created by Toolbar)\n    in Toolbar (created by Connect(Toolbar))\n    in Connect(Toolbar) (created by App)\n    in div (created by App)\n    in App (created by Connect(App))\n    in Connect(App)\n    in Provider"
[Child 9812, Main Thread] WARNING: NS_ENSURE_TRUE(mPresShell) failed: file /builds/worker/checkouts/gecko/layout/base/nsPresContext.cpp, line 839

Comment 4

[Julian Descottes [:jdescottes]]

Can you check if you have enabled devtools.responsive.browserUI.enabled in about:config? It should be set to false to reproduce the issue.

Comment 5

[Julian Descottes [:jdescottes]]

I see that the preference's default value was just changed to true in https://bugzilla.mozilla.org/show_bug.cgi?id=1621306
That's probably why you are not seeing the issue. Make sure to switch the preference to false before testing.

Comment 6

[Maria Ortiz [ :marylicious ]]

You are right! that was the problem. I'll push in a couple of hours when I get home

Comment 7

[iwshen11]

Hi,

I realize that Maria has already done some work on this bug, but would it be fine if I try my hand at this bug as an exercise to get my development environment setup?

Thanks,
Ivan

Comment 8

[Maria Ortiz [ :marylicious ]]

Attachment: Bug 1620932 Fix React warning in devtools/client/responsive/components/Browser.js

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/48b9d2c8b239
Fix React warning in devtools/client/responsive/components/Browser.js r=gl

Comment 10

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/48b9d2c8b239