The QR code dialog should hint at zooming the dialog in case it's too small

RESOLVED FIXED in Firefox 47

Status

()

Firefox
Developer Tools: WebIDE
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: past, Assigned: simplyblue24, Mentored)

Tracking

Trunk
Firefox 47
Points:
---

Firefox Tracking Flags

(firefox44 affected, firefox47 fixed)

Details

(Whiteboard: [good first bug][lang=html])

Attachments

(1 attachment, 3 obsolete attachments)

In some systems (usually Linux, high resolution) the QR code can appear too small for a good scan by the phone. An easy workaround is to zoom in on the page that displays the dialog or perhaps resize it (if the QR code is sized as a percentage of the window). We could mention that possibility in the dialog text, with something like "If the QR code appears too small for the connection to be successfully established, try zooming or enlarging the window". Someone should probably verify which approach works best and only mention that one.
Hmm, is there a CSS change we make to improve it by default?  Or the window is just too small?  I'm not sure I follow what high res has to do with it yet.

I don't know if I can create a HiDPI Linux easily from within a VM...
Flags: needinfo?(past)
(Reporter)

Comment 2

2 years ago
I'm mostly guessing from my recollection of that person's screen at the time. I *think* he was running Fedora on a seemingly high-res screen, so I assume that had something to do with it.

The fact that I don't know more details is what actually led me to suggest a textual hint instead of ensuring the QR code was bigger, which would naturally be the best thing to do.
Flags: needinfo?(past)
Mentor: jryans@gmail.com
Whiteboard: [good first bug][lang=html]

Comment 3

2 years ago
Hi, would it be okay to work on this bug as my first bug?
How should I get started? I have downloaded the Mozilla build and managed to get the nightly build running.

Thanks

Comment 4

2 years ago
Hi! I would to work on this bug.

I have been able to open WebIDE but not sure how to get a QR code or which file to use?
Flags: needinfo?(jryans)
Thanks for the interest in contributing here!  Sorry for the delay, I was out on PTO for a bit.  Either of you are welcome to give it a try.  I'll assign the bug to whoever attaches a patch.

See the DevTools hacking page[1] if you need help getting started with the code base.

The dialog is displayed as part of connecting to a Firefox OS[2] or Firefox for Android[3] device via WiFi.  However, I bet you could still work on this bug even without those devices.

While the actual dialog appears attached to the WebIDE window, it's possible to display a basic version of its content at chrome://webide/content/wifi-auth.xhtml in a new tab.  (The blank space is where the QR code is shown during the real workflow.)

At the moment, I'm guessing we'd like to add a text hint as mentioned in comment 0, probably near the bottom of the page.

[1]: https://wiki.mozilla.org/DevTools/Hacking
[2]: https://developer.mozilla.org/en-US/docs/Tools/Remote_Debugging/Debugging_Firefox_OS_over_Wifi
[3]: https://developer.mozilla.org/en-US/docs/Tools/Remote_Debugging/Debugging_Firefox_for_Android_over_Wifi
Flags: needinfo?(jryans)

Comment 6

2 years ago
Hello, I will start working on this bug.
(Assignee)

Comment 7

2 years ago
Created attachment 8712605 [details] [diff] [review]
1209417.patch

Hi,please review the patch. Thanks !!
Attachment #8712605 - Flags: review?(jryans)
Assignee: nobody → bmanojkumar24
Status: NEW → ASSIGNED
Comment on attachment 8712605 [details] [diff] [review]
1209417.patch

Review of attachment 8712605 [details] [diff] [review]:
-----------------------------------------------------------------

Great, thanks for working on it!  Let's move it to a DTD file, and it should be ready.

Also, please add "r=jryans" at the end of your commit message.

::: devtools/client/webide/content/wifi-auth.xhtml
@@ +38,5 @@
>        <a id="yes-scanner" class="toggle-scanner">&wifi_auth_yes_scanner;</a>
>      </div>
>  
> +    <div>
> +      <h5>If the QR code appears too small for the connection to be successfully established, try zooming or enlarging the window</h5>

Add a period at the end of the sentence.

Also, we should move this to a DTD file so it can be translated.  Add a new entity[1] to webide.dtd, such as "wifi_auth_qr_size_note" or something.  Then you can refer to it here with "&wifi_auth_qr_size_note;" like others above.

[1]: https://dxr.mozilla.org/mozilla-central/source/devtools/client/locales/en-US/webide.dtd
Attachment #8712605 - Flags: review?(jryans) → feedback+
(Assignee)

Comment 9

2 years ago
Created attachment 8713037 [details] [diff] [review]
1209417.patch

Hope this is looking good now :)
Attachment #8712605 - Attachment is obsolete: true
Attachment #8713037 - Flags: review?(jryans)
(Assignee)

Comment 10

2 years ago
Created attachment 8713043 [details] [diff] [review]
1209417.patch

Please review.Thanks.
Attachment #8713037 - Attachment is obsolete: true
Attachment #8713037 - Flags: review?(jryans)
Attachment #8713043 - Flags: review?(jryans)
Comment on attachment 8713043 [details] [diff] [review]
1209417.patch

Review of attachment 8713043 [details] [diff] [review]:
-----------------------------------------------------------------

Just a few more tweaks I think we should make.

Let's center the note.  Give the new <div> some ID, and use CSS in wifi-auth.css to set "text-align: center" or similar.  (For easier debugging, you can open "chrome://webide/content/wifi-auth.xhtml" in a Firefox tab and use the DevTools.)

Also, we have the "No QR scanner prompt?" link, which hides the QR code.  In JS, we set the attribute "token" on the body when the token is displayed (instead of the QR code).  We should add additional CSS so that the new note is hidden when body[token] matches.
Attachment #8713043 - Flags: review?(jryans) → feedback+
(Assignee)

Comment 12

2 years ago
Created attachment 8713980 [details] [diff] [review]
1209417.patch

Hi, I have made the changes,Hope they look good.
Attachment #8713043 - Attachment is obsolete: true
Attachment #8713980 - Flags: review?(jryans)
Comment on attachment 8713980 [details] [diff] [review]
1209417.patch

Review of attachment 8713980 [details] [diff] [review]:
-----------------------------------------------------------------

Great, I think that works well!  I'll push this to try to ensure all tests are still passing.

Thanks for working on this!
Attachment #8713980 - Flags: review?(jryans) → review+
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=98717f9144eb
Keywords: checkin-needed

Comment 15

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/3ea568ea22f6
Keywords: checkin-needed

Comment 16

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3ea568ea22f6
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
You need to log in before you can comment on or make changes to this bug.