732688 - No Help button in the Page Info window

Status: VERIFIED FIXED

(Firefox :: Page Info Window, enhancement)

Description

[antistress]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120302 Firefox/13.0a1
Build ID: 20120302031112

Steps to reproduce:

Go to Site Identity panel and try to tweak it.


Actual results:

No Help button


Expected results:

Site Identity panel should provide a Help button just like the Preferences Panel

Comment 1

[antistress]

Maybe that Bug 588685 (Update Site Identity Panel) could be seen like a parent bug

Comment 2

[antistress]

Just to be clear : i'm talking about the Page Info window https://support.mozilla.org/en-US/kb/Page%20Info%20window?s=site+identity&r=6&e=sph&as=s

Comment 3

[Ioana (away)]

Marking this as an enhancement request since the current behavior is by design.

Comment 4

[Abhinav Koppula]

Attachment: pageInfoHelp.patch

Comment 5

[Abhinav Koppula]

Attachment: For UI Review

Comment 6

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

Comment on attachment 8567347 [details] [diff] [review]
pageInfoHelp.patch

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

::: browser/base/content/pageinfo/pageInfo.xul
@@ +411,4 @@
>      <!-- Others added by overlay -->
>    </deck>
>  
> +  <hbox id="helpPanel" pack="end">

This ID is confusing since this is not a panel but an hbox.

@@ +411,5 @@
>      <!-- Others added by overlay -->
>    </deck>
>  
> +  <hbox id="helpPanel" pack="end">
> +      <label onclick="doHelpButton();" class="text-link" value="&helpButton.label;" />

This should be using a button instead of a label.

Comment 7

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

Comment on attachment 8567351 [details]
For UI Review

The "Help" link here looks out of place. There is not enough distance between it and the edges of the dialog. A button would probably look better.

Comment 8

[Abhinav Koppula]

Attachment: pageInfo.patch

Added Help button instead of label

Comment 9

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

Attachment: Patch

Abhinav, what do you think about this approach?

I moved the help button inside of each section and then we don't need to adjust any styling to get it positioned correctly relative to the contents of each section.

It also fixes the issue that I saw where the button was getting clipped on smaller windows (still possible if the window is resized but this issue existed before this patch).

Comment 10

[Abhinav Koppula]

Yes Jared, this looks like a good approach since we need not add any custom styling. Thanks 

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #9)
> Created attachment 8569401 [details] [diff] [review]
> Patch
> 
> Abhinav, what do you think about this approach?
> 
> I moved the help button inside of each section and then we don't need to
> adjust any styling to get it positioned correctly relative to the contents
> of each section.
> 
> It also fixes the issue that I saw where the button was getting clipped on
> smaller windows (still possible if the window is resized but this issue
> existed before this patch).

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #9)
> Created attachment 8569401 [details] [diff] [review]
> Patch
> 
> Abhinav, what do you think about this approach?
> 
> I moved the help button inside of each section and then we don't need to
> adjust any styling to get it positioned correctly relative to the contents
> of each section.
> 
> It also fixes the issue that I saw where the button was getting clipped on
> smaller windows (still possible if the window is resized but this issue
> existed before this patch).

Comment 11

[:Gijs (he/him)]

Comment on attachment 8569401 [details] [diff] [review]
Patch

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

::: browser/base/content/pageinfo/pageInfo.xul
@@ +155,5 @@
>            <treechildren id="metatreechildren" flex="1"/>
>          </tree>        
>        </groupbox>
> +      <hbox pack="end">
> +        <button oncommand="doHelpButton()" label="&helpButton.label;" />

s/oncommand="doHelpButton()"/command="cmd_help"/g

and add:

dlgtype="help"

so that it doesn't look like ... on mac, please. :-)

Comment 12

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

Attachment: Patch for checkin

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/95f54090bb24

Comment 14

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

https://hg.mozilla.org/mozilla-central/rev/95f54090bb24

Comment 15

[Saheda Reza [:Antora]]

I have successfully reproduce this bug on  Firebox nightly 13.0a1 (2012-03-03).
I have successfully tested the fixed in this builds and fix works : 
Firefox beta 39.0 build id: 20150618135210 User Agent 	Mozilla/5.0 (Windows NT 6.3; rv:39.0) Gecko/20100101 Firefox/39.0

Comment 16

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

Thanks!