920279 - Reduce width of about:firefox which is currently wider than the viewport

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Paul [pwd]]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:27.0) Gecko/20100101 Firefox/27.0 (Beta/Release)
Build ID: 20130923060712

Steps to reproduce:

As part of the fallout from bug 705246 landing, you can notice that about:firefox is wider than that of the viewport causing the page to wiggle somewhat upon being touched.

Comment 1

[Aaron Train [:aaronmt]]

Perhaps this can be a mentor bug?

Comment 2

[:Margaret Leibovic]

What device is this happening on? I can't reproduce locally.

Comment 3

[Aaron Train [:aaronmt]]

I'm able to reproduce on any device, it's very subtle shifting if you try and touch content and pan it side to side

Comment 4

[:Margaret Leibovic]

Oh, I wasn't looking closely enough :)

I can help mentor this bug. The xhtml file for this page is located here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/about.xhtml

Instead of setting a width and scale, we should just use width=device-width, similarly to how we do in other files. 

I did a search for all the <meta name="viewport"> tags we use for chrome pages, and it looks like we should update config.xhtml as well:
http://mxr.mozilla.org/mozilla-central/search?string=meta+name%3D%22viewport&find=%2Fmobile%2Fandroid%2Fchrome&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central

Comment 5

[Jayesh]

Hey Margaret m ready to solve this bug. Please do guide me to this.

Comment 6

[:Margaret Leibovic]

(In reply to Jayesh from comment #5)
> Hey Margaret m ready to solve this bug. Please do guide me to this.

Hi Jayesh! First question: do you have a build environment set up for Fennec? If not, you'll want to follow the directions here:
https://wiki.mozilla.org/Mobile/Fennec/Android

Once you have a local build and can test changes, you should try modifying the files I pointed to in comment 4. Then you can follow the directions here to generate and upload a patch:
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

If you have more questions, you're always welcome to ask them in #mobile on irc.mozilla.org. Also see our Get Involved page for more resources on getting started:
https://wiki.mozilla.org/Mobile/Get_Involved

Comment 7

[bhavana bajaj [:bajaj]]

Unclear why this would be tracked as it seems to be a minor polish issue. I am happy to take a low risk uplift here when ready but may not be tracking worthy.Erin, do you agree ?

Do we have a screenshot or video showing how bad the issue is ?

Does it just happen on about:firefox or any page ?

Comment 8

[Guylaine]

Hi! I'm Guylaine, just want to know how this bug works? Don't know where to start please help.

Comment 9

[Aaron Train [:aaronmt]]

Guylaine, this bug already has a supposed assignee. If you're interested in other bugs there are many available. As for this bug, the suggested instructions are in comment #4 and comment #6 for your curiosity.

Comment 10

[:Margaret Leibovic]

Jayesh, how is work on this bug coming along? Do you need any help?

If you're no longer working on it, just let us know, and we can let other newcomers give it a shot.

Comment 11

[Jayesh]

hey margaret, i have started working on this bug and right now going through some initial set ups for android sdk.

Comment 12

[:Margaret Leibovic]

(In reply to Jayesh from comment #11)
> hey margaret, i have started working on this bug and right now going through
> some initial set ups for android sdk.

Sounds good! Sorry for unassigning you, I thought when you cleared the needinfo? flag without a comment, you were indicating that you weren't working on this anymore.

Please let me know if there's anything I can help you with. Setting up a build environment shouldn't be *too* hard, so if you're running into errors, definitely let us know.

Comment 13

[Jayesh]

Attachment: Patch for width

Please Review

Comment 14

[:Margaret Leibovic]

Comment on attachment 827942 [details] [diff] [review]
Patch for width

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

Thanks for the patch! Unfortunately, I was wrong about the fix needed here. Did you test this patch out? It looks like the about:firefox layout gets messed up, so we'll probably need some CSS tweaks to go along with this viewport change.

This is the file that we'll need to edit:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/themes/core/aboutPage.css

I actually wonder if a quicker fix would be to see if there's some CSS mistake that's making the page take up more than 480px, since that would probably lead to the wider-than-the-viewport problem we're seeing.

Comment 15

[Jayesh]

Hey Margaret, I will surely have a look at this.

Thanks
Jayesh

Comment 16

[Tan]

Hey, I'm Tan.  

I'd like to start working on this if work isn't being done on it.

Comment 17

[:Margaret Leibovic]

(In reply to Tan from comment #16)
> Hey, I'm Tan.  
> 
> I'd like to start working on this if work isn't being done on it.

Hi Tan! First of all, do you have a Fennec development environment set up? If not, you'll want to follow the directions here:
https://wiki.mozilla.org/Mobile/Fennec/Android

It's been a while since I've looked at this bug, but in comment 14 I mentioned that we should look into aboutPage.css, to see if there is a CSS error.

Using the remote inspector would also probably help debug this issue, I would encourage you to try that out:
https://developer.mozilla.org/en-US/docs/Tools/Remote_Debugging/Firefox_for_Android
https://developer.mozilla.org/en-US/docs/Tools/Page_Inspector

Comment 18

[Tan]

Hey Margaret!

I've got a dev enviroment up and I've popped out an APK. Thank you for the links to the remote inspector. I'm looking into it now.

Comment 19

[Tan]

Attachment: Fix for about:firefox width

Comment 20

[Tan]

Comment on attachment 8365611 [details] [diff] [review]
Fix for about:firefox width

# HG changeset patch
# Parent cdc0ab2c0cbad488f517812a3cf1694eeddaa84f
# User Tan <[e-mail]>
Bug 920279
Margin-left and margin-right for banner and aboutLinks were negative. Changed margin-left and margin-right to 0px.

diff --git a/mobile/android/themes/core/aboutPage.css b/mobile/android/themes/core/aboutPage.css
--- a/mobile/android/themes/core/aboutPage.css
+++ b/mobile/android/themes/core/aboutPage.css
@@ -22,17 +22,16 @@ body {
 }
 
 #version {
   margin: 0 0 0 15px;
   font-size: 15px;
 }
 
 #banner {
-  margin: 0 -10px;
   min-height: 150px;
   background-color: #bdc7ce;
 }
 
 #logo {
   position: absolute;
   top: 0;
   right: 0;
@@ -74,17 +73,17 @@ body {
   text-align: center;
 }
 
 #telemetry a {
   text-decoration: underline;
 }
 
 #aboutLinks {
-  margin: 0 -10px 15px -10px;
+  margin: 0 0 15px 0;
   padding: 0;
 }
 
 #aboutLinks > li {
   line-height: 2.6;
   border-top: 1px solid white;
   border-bottom: 1px solid #C1C7CC;
 }

Comment 21

[Tan]

Comment on attachment 8365611 [details] [diff] [review]
Fix for about:firefox width

># HG changeset patch
># Parent cdc0ab2c0cbad488f517812a3cf1694eeddaa84f
># User Tan <[e-mail]>
>Bug 920279
>Margin-left and margin-right for banner and aboutLinks were negative. Changed margin-left and margin-right to 0px.
>
diff --git a/mobile/android/themes/core/aboutPage.css b/mobile/android/themes/core/aboutPage.css
--- a/mobile/android/themes/core/aboutPage.css
+++ b/mobile/android/themes/core/aboutPage.css
@@ -22,17 +22,16 @@ body {
 }
 
 #version {
   margin: 0 0 0 15px;
   font-size: 15px;
 }
 
 #banner {
-  margin: 0 -10px;
   min-height: 150px;
   background-color: #bdc7ce;
 }
 
 #logo {
   position: absolute;
   top: 0;
   right: 0;
@@ -74,17 +73,17 @@ body {
   text-align: center;
 }
 
 #telemetry a {
   text-decoration: underline;
 }
 
 #aboutLinks {
-  margin: 0 -10px 15px -10px;
+  margin: 0 0 15px 0;
   padding: 0;
 }
 
 #aboutLinks > li {
   line-height: 2.6;
   border-top: 1px solid white;
   border-bottom: 1px solid #C1C7CC;
 }

Comment 22

[Tan]

Attachment: Fix for about:firefox width 2

Comment 23

[Tan]

Sorry about the many posts. I'm still new to bugzilla and I'm not sure how to delete comments.

Comment 24

[:Margaret Leibovic]

Comment on attachment 8365613 [details] [diff] [review]
Fix for about:firefox width 2

Thanks for working on this! When posting a patch, you should request review from an appropriate reviewer with the review? flag. Since I'm the right reviewer for this, I'll set it to myself, and I'll take a closer look when I get a chance later :)

Comment 25

[:Margaret Leibovic]

Comment on attachment 8365613 [details] [diff] [review]
Fix for about:firefox width 2

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

This works well, thanks!

Comment 26

[:Margaret Leibovic]

I'm marking this bug as checkin-needed, so someone will come along and land it for you!

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/32c30f251976

Comment 28

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/32c30f251976