378770 - Clean up Security Page Info visuals

Status: RESOLVED FIXED

(Firefox :: Page Info Window, defect)

Description

[Johnathan Nightingale [:johnath]]

Filed as a result of discussion and review comments on bug 377076, this bug tracks the need for a patch on the new page info page which fixes some visual problems, specifically:

 - Use groupboxes instead of groove separators, to bring it into consistency with the rest of page info
 - Fix style bugs (especially on windows platform).  Michael Ventor has posted a patch on the original bug that I will move here.
 - Clean up the view certificate button, which presents differently than the other buttons on the page.

Comment 1

[Michael Ventnor]

Attachment: Windows follow-ups from 377076

- Makes dialog readable on Windows
- Fixes oversight where readonly attributes weren't on new textfields
- Improves i18n of view count

Comment 2

[Michael Ventnor]

One other suggestion:

"Have I visited this website before today?" just doesn't sound right IMO. A much clearer way to put it is "Have I visited this website prior to today?"

Comment 4

[Johnathan Nightingale [:johnath]]

Attachment: First patch for cleanup

Subsumes Ventnor's patch and includes the change to groupboxes and the re-org of the view cert button.  

Michael, before I flag this patch for review, can you have a look and see how it behaves in windows, and whether I broke anything from your patch?

Comment 5

[Michael Ventnor]

Everything looks fine here.

Comment 6

[Johnathan Nightingale [:johnath]]

Attachment: Updated page info screencap

Includes re-org'd view cert button, groupbox treatment consistent with the rest of page info, and the windows breakage cleanups, as verified by Michael Ventnor

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 264260 [details]
Updated page info screencap

Bam!

Comment 8

[Johnathan Nightingale [:johnath]]

Comment on attachment 264165 [details] [diff] [review]
First patch for cleanup

Reviewer note: a lot of the delta in pageInfo.xul is just indenting because of the groupbox wrapping.  Sorry about that.  :(

Comment 9

[Asaf Romano (gone)]

Comment on attachment 264165 [details] [diff] [review]
First patch for cleanup

-<!ENTITY  securityView.label    "View">
+<!ENTITY  securityView.label    "View Certificate">

rev the entity name, else localizers are not notified on this change.

Please also remove all these trailing spaces from the xul file (in places you're patching).

r=mano otherwise.

Comment 10

[Johnathan Nightingale [:johnath]]

Attachment: Patch with Mano's review comments

Dropped trailing lines and tweaked entity name.

Comment 11

[Florian Quèze [:florian]]

Comment on attachment 264465 [details] [diff] [review]
Patch with Mano's review comments

There are still some trailing spaces in the xul file:

>+          <button id="security-view-cert" label="&securityView.certView;"
>+                  accesskey="&securityView.accesskey;"
>+                  oncommand="security.viewCert();"/>          

>+        <caption id="security-privacy" label="&securityView.privacy.header;" />
>+        <grid>
>+          <columns>
>+            <column flex="1"/>
>+            <column flex="1"/>
>+          </columns>          

>+          </rows>
>+        </grid>
>+      </groupbox>

Comment 12

[Johnathan Nightingale [:johnath]]

Attachment: Clean up wayward whitespace

Thanks, Florian

Comment 13

[Nickolay_Ponomarev]

mozilla/browser/base/content/pageinfo/pageInfo.xul                1.4
mozilla/browser/base/content/pageinfo/security.js                 1.4
mozilla/browser/locales/en-US/chrome/browser/pageInfo.dtd         1.13
mozilla/browser/locales/en-US/chrome/browser/pageInfo.properties  1.9
mozilla/browser/themes/pinstripe/browser/pageInfo.css             1.9
mozilla/browser/themes/winstripe/browser/pageInfo.css             1.9