774500 - Make about:support more mobile-friendly

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[:Margaret Leibovic]

To start, the "Copy all to clipboard" button needs to be made wider. After that, we should make the text in the tables larger so that it's easier to read.

Comment 1

[Mark Capella [:capella]]

Attachment: Patch (v1)

This is interesting ... on my Galaxy device, Fennec 14.0 works properly, but when I tried the about:support page under nightly / Fennec 17.01a, the button was indeed too small for the text.

It's created as a three line button:
"  Copy "
" all to"
"clipboard"

I can't explain it, but I can fix it with this quick patch, which will re-size the button with a better width so the whole thing takes one line again.

Comment 2

[Mark Capella [:capella]]

Attachment: Patch (v2)

Forgot to qref ...

Comment 4

[:Margaret Leibovic]

Comment on attachment 654073 [details] [diff] [review]
Patch (v2)

Thanks for writing a patch, Mark.

I don't think that we should take this approach just to fix the button, since it won't scale well to fixing other styles on the page. I think a better approach would be to use our own CSS file for mobile. We already override some toolkit CSS files like so:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/themes/core/jar.mn#20

Adding an id to the toolkit HTML won't affect other platforms, so it would be fine to still add that. However, in the future we may want to override the whole HTML file to create a more mobile-specific UI by including things like a <meta name="viewport"> tag.

Comment 5

[Mark Capella [:capella]]

Attachment: Patch (v3)

Aha! Thanks, I didn't know the mechanism for overiding modules, but that definitely simplifies things.

This new patch:

  Removes Hidden Reset-Box for better title display / rendering
  Sets width for "Copy to clipboard" button
  Provides Haptic feedback for "Copy to clipboard" button
  Removes unused Profile Folder / Show Folder Button table entry
  Enlarges Basic Text for Readability

Comment 6

[:Margaret Leibovic]

Comment on attachment 654544 [details] [diff] [review]
Patch (v3)

This looks like a much better approach. However, I'd like to see if we can avoid overriding the HTML file. We've run into problems in the past where we fork a mobile version of a toolkit page, and then we miss out on updates to original page. Since toolkit is supposed to be a shared resource, we can add ids to elements to style them as needed in our own CSS. You mentioned that the hidden reset-box was causing problems, but there should be something we can do in CSS to fix that.

>diff --git a/mobile/android/chrome/content/aboutSupport.xhtml b/mobile/android/chrome/content/aboutSupport.xhtml

>+      <button style="width: 500px"

You should do this with CSS. Also, using a hard-coded pixel value might not work well on all devices. If we just want this button to extend across the screen, we could do |width: 100%;|

>+        onclick="copyContentsToClipboard();
>+                 Haptic.performSimpleAction(Haptic.LongPress);">

I'm not sure that we need haptic feedback here, since regular buttons on the web don't have it.

Comment 7

[Mark Capella [:capella]]

Attachment: Patch (v4)

Ok... maybe this version?

Comment 8

[:Margaret Leibovic]

Comment on attachment 654949 [details] [diff] [review]
Patch (v4)

Overall looking good. I just want to make sure the button styling is right before giving this an r+.

Also, I found a worse problem than the styling of this page - copying to the clipboard isn't working. We should file another bug on that, and if it sounds like something you're interested in, it would be great to fix that!

>diff --git a/toolkit/content/aboutSupport.xhtml b/toolkit/content/aboutSupport.xhtml

>     <div>
>-      <button onclick="copyContentsToClipboard()">
>+      <button id="button-copyToClipboard" onclick="copyContentsToClipboard()">

When I tested this, the button was still squished. It looks like setting width:100%; on the button isn't enough because of its parent div. I think we'll need to set both the button and its parent div to width:100%;. What we can do is just set an id on the div (something like "copy-to-clipboard" to be more consistent with the way ids are named in the rest of this file), then set a style on |#copy-to-clipboard|, as well as |#copy-to-clipboard > button|. 

And maybe we can throw some padding on that button, too :)

>-          <tr>
>+          <tr id="row-appBasicsProfile">

Nit: Don't use camel case in the id, to be consistent with our id naming conventions. Maybe something like "profile-row" would be better for this id (we don't need to worry too much about id collision, since this is a self-contained HTML file).

Comment 9

[Mark Capella [:capella]]

  Application Basics

        Name
        Fennec

        Version
        17.0a1

        User Agent
        Mozilla/5.0 (Android; Mobile; rv:17.0) Gecko/17.0 Firefox/17.0

        Profile Directory

          Open Directory

        Enabled Plugins

          about:plugins

        Build Configuration

          about:buildconfig

        Crash Reports

          about:crashes

        Memory Use

          about:memory

  Extensions

        Name

        Version

        Enabled

        ID

        Adblock Plus
        2.1.2
        true
        {d10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d}

  Important Modified Preferences

      Name

      Value

        browser.cache.disk.capacity
        204800

        browser.cache.disk.smart_size.first_run
        false

        browser.cache.disk.smart_size_cached_value
        204800

        browser.startup.homepage_override.mstone
        17.0a1

        dom.mozApps.used
        true

        extensions.lastAppVersion
        17.0a1

        network.cookie.prefsMigrated
        true

  Graphics

        Adapter Description
        Qualcomm -- Adreno (TM) 225 -- OpenGL ES 2.0 2184622 -- Model: SPH-L710, Product: d2spr, Manufacturer: samsung, Hardware: SPH-L710

        Vendor ID
        Qualcomm

        Device ID
        Adreno (TM) 225

        Driver Version
        OpenGL ES 2.0 2184622

        WebGL Renderer
        Qualcomm -- Adreno (TM) 225 -- OpenGL ES 2.0 2184622

        GPU Accelerated Windows
        0

        AzureCanvasBackend
        cairo

        AzureFallbackCanvasBackend
        none

        AzureContentBackend
        none

  JavaScript

        Incremental GC
        1

  Accessibility

        Activated
        1

        Prevent Accessibility
        0

  Library Versions

        Expected minimum version

        Version in use

        NSPR
        4.9.2
        4.9.2

        NSS
        3.13.6.0 Basic ECC
        3.13.6.0 Basic ECC

        NSS Util
        3.13.6.0
        3.13.6.0

        NSS SSL

        3.13.6.0 Basic ECC
        3.13.6.0 Basic ECC

        NSS S/MIME
        3.13.6.0 Basic ECC
        3.13.6.0 Basic ECC

Comment 10

[Mark Capella [:capella]]

Attachment: Patch (v5)

Odd... I wonder if you can check this again ... ? (new element ID's have been changed to avoid camel-notation). The copy-to-clipboard function works for me, above is attached the paste of my Galaxy-S3 info.

Also, the button appears properly wide (and with padding now) without further modification at the DIV level.

Comment 11

[Mark Capella [:capella]]

Comment on attachment 655478 [details] [diff] [review]
Patch (v5)

Forgot to flag this ...

Comment 12

[:Margaret Leibovic]

(In reply to Mark Capella [:capella] from comment #10)

> Odd... I wonder if you can check this again ... ? (new element ID's have
> been changed to avoid camel-notation). The copy-to-clipboard function works
> for me, above is attached the paste of my Galaxy-S3 info.

Hm, it did work for me now. Maybe I was testing some build that had touch event problems and the button wasn't getting clicked properly? In any case, works now :)

Comment 13

[:Margaret Leibovic]

Comment on attachment 655478 [details] [diff] [review]
Patch (v5)

This looks great to me.

Comment 14

[Mark Capella [:capella]]

Comment on attachment 655478 [details] [diff] [review]
Patch (v5)

Ok then, quick request for review? then I'll move it along.

Comment 15

[Mark Capella [:capella]]

Start with TRY:
https://tbpl.mozilla.org/?tree=Try&rev=261cc153eb71

Comment 16

[Mark Capella [:capella]]

On to inbound:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=3e83ba39ec95

Comment 17

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/3e83ba39ec95

Comment 18

[Cristian Nicolae (:xti)]

Changes were applied for about:support page on the latest Nightly. Closing bug as verified fixed on:

Firefox 18.0a1 (2012-09-17)
Device: Galaxy Note
OS: Android 4.0.4