1291488 - "Learn More" text on About dialog deprecating OSX 10.7 is misaligned

Status: RESOLVED FIXED

(Firefox :: Theme, defect, P4)

Description

[Stuart Dunkeld]

Attachment: Screen Shot 2016-08-03 at 00.20.07.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:48.0) Gecko/20100101 Firefox/48.0
Build ID: 20160726073904

Steps to reproduce:

Open "About Firefox" dialog in Firefox 48 on OSX Lion


Actual results:

The "Learn More" label next to the text "You cannot perform further updates on this system" is not correctly aligned with the text.


Expected results:

The label should be correctly aligned with the text.

Comment 1

[Tooru Fujisawa [:arai]]

The text is here.

https://dxr.mozilla.org/mozilla-central/rev/ffac2798999c5b84f1b4605a1280994bb665a406/browser/base/content/aboutDialog.xul#113
>               <hbox id="unsupportedSystem" align="center">
>                 <label>&update.unsupported.start;</label><label id="unsupportedLink" class="text-link">&update.unsupported.linkText;</label><label>&update.unsupported.end;</label>
>               </hbox>


label element has margin by default

https://dxr.mozilla.org/mozilla-central/rev/ffac2798999c5b84f1b4605a1280994bb665a406/toolkit/themes/osx/global/global.css#176
> description,
> label {
>   cursor: default;
>   margin-top: 1px;
>   margin-bottom: 2px;
>   margin-inline-start: 6px;
>   margin-inline-end: 5px;
> }


but text-link class removes the margin, and it results in misaligned text.

https://dxr.mozilla.org/mozilla-central/rev/ffac2798999c5b84f1b4605a1280994bb665a406/browser/base/content/aboutDialog.css#83
> .text-link,
> .text-link:focus {
>   margin: 0px;
>   padding: 0px;
> }


This issue happens also with other entries in updateDeck, like downloadFailed and manualUpdate.

Comment 2

[Dão Gottwald [:dao]]

(In reply to Tooru Fujisawa [:arai] from comment #1)
> This issue happens also with other entries in updateDeck, like
> downloadFailed and manualUpdate.

Is this only on OS X or across platforms?

Comment 3

[Tooru Fujisawa [:arai]]

(In reply to Dão Gottwald [:dao] from comment #2)
> (In reply to Tooru Fujisawa [:arai] from comment #1)
> > This issue happens also with other entries in updateDeck, like
> > downloadFailed and manualUpdate.
> 
> Is this only on OS X or across platforms?

Sorry, I just meant, there are same cases that label and text-link label are in same line inside hbox, in aboutDialog.xul.

https://dxr.mozilla.org/mozilla-central/rev/ffac2798999c5b84f1b4605a1280994bb665a406/browser/base/content/aboutDialog.xul#97
>               <hbox id="downloadFailed" align="center">
>                 <label>&update.failed.start;</label><label id="failedLink" class="text-link">&update.failed.linkText;</label><label>&update.failed.end;</label>
>               </hbox>

https://dxr.mozilla.org/mozilla-central/rev/ffac2798999c5b84f1b4605a1280994bb665a406/browser/base/content/aboutDialog.xul#109
>               <hbox id="manualUpdate" align="center">
>                 <label>&update.manual.start;</label><label id="manualLink" class="text-link"/><label>&update.manual.end;</label>
>               </hbox>


I haven't yet checked when or on which platform those hboxes are shown.
but I think those are not platform-specific.

Comment 4

[Dão Gottwald [:dao]]

Right, those labels themselves aren't platform-specific, but it would be interesting to know if this bug is platform-specific.

Comment 5

[Tooru Fujisawa [:arai]]

okay, will check the rendering result on each OS shortly.

Comment 6

[Dão Gottwald [:dao]]

Thank you!

Comment 7

[Tooru Fujisawa [:arai]]

Attachment: About Firefox on each platform

yeah, you're right.
interestingly the misalignment happens only on OSX.
maybe there's other style that affects the alignment, other than margin?

I already confirmed removing the "marign: 0" rule fixes the misalignment, on OSX.

Comment 8

[Dão Gottwald [:dao]]

(In reply to Tooru Fujisawa [:arai] from comment #7)
> I already confirmed removing the "marign: 0" rule fixes the misalignment, on
> OSX.

Would you like to submit a patch for that?

Comment 9

[Tooru Fujisawa [:arai]]

Sure :)

According to the patch that adds the rule, it seems to be for text-link inside description,
https://hg.mozilla.org/mozilla-central/rev/87590083255c
so I'll change the selector to "description .text-link".

Comment 10

[Tooru Fujisawa [:arai]]

Attachment: Do not apply |margin: 0px| and |padding: 0px| to text-link outside of description in about dialog.

changed selector |.text-link| to |description .text-link|, so that the rule is applied only to text link inside description, and text-link next to normal label won't be affected by the rule.

Comment 11

[Dão Gottwald [:dao]]

Comment on attachment 8780242 [details] [diff] [review]
Do not apply |margin: 0px| and |padding: 0px| to text-link outside of description in about dialog.

Looks good, but please use the child selector: description > .text-link

Thanks!

Comment 12

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5b9a61e79b77412b0969cadb04290697ea9ebb38
Bug 1291488 - Do not apply |margin: 0px| and |padding: 0px| to text-link outside of description in about dialog. r=dao

Comment 13

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

https://hg.mozilla.org/mozilla-central/rev/5b9a61e79b77