1130741 - Sync pane "Terms of Service" & "Privacy notice" links are too small

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P4)

Description

[Tim Nguyen :ntim]

Screenshot : http://i.imgur.com/2RUP3p1.png

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

This isn't so much of a regression from the windowed preferences, but it would still be nice to fix since in-content preferences are new. However, we won't block shipping for this.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Was this fixed by the work in bug 1136670?

Comment 3

[Tim Nguyen :ntim]

The alignment is fixed, but not the small font size.

Comment 4

[Tim Nguyen :ntim]

To fix this bug :
In [0], remove the small class from the links. There also needs to be more margin on top and bottom of these links, but I think it's better to handle that later, since this is a bit tricky on windows.


[0] : http://mxr.mozilla.org/mozilla-central/source/browser/components/preferences/in-content/sync.xul#313

Comment 5

[Léon McGregor]

I would like to take this bug. I'm still new to working with the source code.

Comment 6

[Tim Nguyen :ntim]

(In reply to Léon McGregor from comment #5)
> I would like to take this bug. I'm still new to working with the source code.
Hi Léon,
Thanks for your interest in this bug.

You can take a look at http://codefirefox.com , it provides great tutorials. Or if you prefer reading written docs : https://developer.mozilla.org/en-US/docs/Simple_Firefox_build

If you need help, you can reach #introduction on IRC (irc.mozilla.org).

Comment 7

[Léon McGregor]

(In reply to Tim Nguyen [:ntim] from comment #4)
> To fix this bug :
> In [0], remove the small class from the links. There also needs to be more
> margin on top and bottom of these links, but I think it's better to handle
> that later, since this is a bit tricky on windows.
> 
> 
> [0] :
> http://mxr.mozilla.org/mozilla-central/source/browser/components/preferences/
> in-content/sync.xul#313

You mentioned changing the margins. Do you think this should be in a separate bug, or should I attempt to fix them at this point as well?

Comment 8

[Léon McGregor]

Attachment: bug1130741_correctTOSTextSize.diff

This removes the "small" classes from the relevant links

Comment 9

[Tim Nguyen :ntim]

Comment on attachment 8573505 [details] [diff] [review]
bug1130741_correctTOSTextSize.diff

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

Looks good to me ! But my review is worth nothing.

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8573505 [details] [diff] [review]
bug1130741_correctTOSTextSize.diff

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

Please update /browser/themes/preferences/*/preferences.css to have a comment at the lines for `label.small` with "These lines should be removed as part of fixing bug 1140495".

With the above changes then it should be ready for r+

Comment 11

[Léon McGregor]

Attachment: bug1130741_correctTOSTextSize.diff - with comments for 1140495

Includes commenting in preferences.css for bug 1140495

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8574106 [details] [diff] [review]
bug1130741_correctTOSTextSize.diff - with comments for 1140495

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

Thanks!

Comment 13

[Léon McGregor]

What steps should I take now? This is the first patch I've made, so I don't have any commit access rights.

Comment 14

[Jared Wein [:jaws] (please needinfo? me)]

Nothing more is needed on your behalf for this bug. Congrats on your first bug :)

I've assigned bug 1115925 for you to work on next. I'm looking forward to your continued contributions :)

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/784963e85352

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/784963e85352

Comment 17

[Camelia Badau [:cbadau], Desktop Test Engineering]

Verified fixed on Windows 7 64bit, Ubuntu 13.10 32bit and Mac OSX 10.9.5 using latest Nightly 39.0a1 (buildID: 20150317030206).

Comment 18

[:Gijs (he/him)]

Comment on attachment 8574106 [details] [diff] [review]
bug1130741_correctTOSTextSize.diff - with comments for 1140495

(requesting approval per discussion with Jared in our meeting)

Approval Request Comment
[Feature/regressing bug #]: n/a
[User impact if declined]: tiny (hard to read) sync links at the bottom of the in-content-prefs sync pane
[Describe test coverage new/current, TreeHerder]: nope, css only
[Risks and why]: very small, removing attributes that trigger CSS that makes them so tiny
[String/UUID change made/needed]: nope

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/545bf6e4e6a0