Closed Bug 920279 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox for Android :: General, defect, minor)

ARM
Android
defect
Not set
minor

Tracking

()

RESOLVED FIXED
Firefox 29
Tracking Status
firefox27 - affected

People

(Reporter: pwd.mozilla, Assigned: dtsdaa)

Details

(Whiteboard: [mentor=margaret][lang=html])

Attachments

(1 file, 2 obsolete files)

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.
OS: Linux → Android
Hardware: x86 → ARM
Perhaps this can be a mentor bug?
Severity: normal → minor
Status: UNCONFIRMED → NEW
Ever confirmed: true
What device is this happening on? I can't reproduce locally.
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
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
Whiteboard: [mentor=margaret][lang=html]
Hey Margaret m ready to solve this bug. Please do guide me to this.
Assignee: nobody → jayesh.choudhari17
Flags: needinfo?(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
Flags: needinfo?(margaret.leibovic)
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 ?
Hi! I'm Guylaine, just want to know how this bug works? Don't know where to start please help.
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.
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.
Flags: needinfo?(jayesh.choudhari17)
Flags: needinfo?(jayesh.choudhari17)
Assignee: jayesh.choudhari17 → nobody
hey margaret, i have started working on this bug and right now going through some initial set ups for android sdk.
(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.
Assignee: nobody → jayesh.choudhari17
Attached patch Patch for width (obsolete) — Splinter Review
Please Review
Attachment #827942 - Flags: review?(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.
Attachment #827942 - Flags: review?(margaret.leibovic) → review-
Hey Margaret, I will surely have a look at this.

Thanks
Jayesh
Hey, I'm Tan.  

I'd like to start working on this if work isn't being done on it.
(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
Assignee: jayesh.choudhari17 → dtsdaa
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.
Attached patch Fix for about:firefox width (obsolete) — Splinter Review
Comment on attachment 8365611 [details] [diff] [review]
Fix for about:firefox width

# HG changeset patch
# Parent cdc0ab2c0cbad488f517812a3cf1694eeddaa84f
# User Tan <dtsdaa@gmail.com>
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 on attachment 8365611 [details] [diff] [review]
Fix for about:firefox width

># HG changeset patch
># Parent cdc0ab2c0cbad488f517812a3cf1694eeddaa84f
># User Tan <dtsdaa@gmail.com>
>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;
 }
Attachment #8365611 - Attachment is obsolete: true
Attachment #8365613 - Flags: review+
Sorry about the many posts. I'm still new to bugzilla and I'm not sure how to delete comments.
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 :)
Attachment #8365613 - Flags: review+ → review?(margaret.leibovic)
Attachment #827942 - Attachment is obsolete: true
Comment on attachment 8365613 [details] [diff] [review]
Fix for about:firefox width 2

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

This works well, thanks!
Attachment #8365613 - Flags: review?(margaret.leibovic) → review+
I'm marking this bug as checkin-needed, so someone will come along and land it for you!
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/32c30f251976
Keywords: checkin-needed
Whiteboard: [mentor=margaret][lang=html] → [mentor=margaret][lang=html][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/32c30f251976
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [mentor=margaret][lang=html][fixed-in-fx-team] → [mentor=margaret][lang=html]
Target Milestone: --- → Firefox 29
You need to log in before you can comment on or make changes to this bug.