1436568 - Make "clear your recent history" a button instead of a link

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P1)

Description

[Johann Hofmann [:johannh]]

See specs in bug 1421690.

Comment 1

[Johann Hofmann [:johannh]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bd8e127d3b97a54393b13506ed8e1f373607b617

Comment 2

[Johann Hofmann [:johannh]]

Attachment: Bug 1436568 - Make "clear your recent history" a button instead of a link.

Review commit: https://reviewboard.mozilla.org/r/221562/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/221562/

Comment 3

[Johann Hofmann [:johannh]]

This patch is based on the one in bug 1432743.

Comment 4

[Johann Hofmann [:johannh]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=01d2ae7a936f578fd58e9362b71916243578d973

Comment 5

[(inactive) Nihanth Subramanya [:nhnt11]]

Comment on attachment 8952310 [details]
Bug 1436568 - Make "clear your recent history" a button instead of a link.

https://reviewboard.mozilla.org/r/221562/#review228546

Looks mostly good to me, thanks! Just one real comment:

Before this patch, when "Never remember history" is selected, we are forcing the time span value in the dialog to "Everything". After this patch, we aren't doing that, and I think it might be confusing for a user who has selected "Never remember" to see stuff like "Last hour"/"2 hours"/"Past day"/whatever. Let's preserve behavior - shouldn't be too complicated to implement.

::: browser/components/preferences/in-content/privacy.xul:99
(Diff revision 1)
>          </menupopup>
>        </menulist>
>      </hbox>
>      <label>&historyHeader.post.label;</label>
>    </hbox>
> -  <deck id="historyPane">
> +  <hbox>

I see that the spec suggests we place the button horizontally next to the pane, but please make sure this doesn't look too crowded when "Use custom settings for history" is selected.

::: browser/components/preferences/in-content/privacy.xul:161
(Diff revision 1)
>  <span class="indent">>></span>      </vbox>
>  <span class="indent">>></span>    </vbox>
>  <span class="indent">>></span>  </deck>
> +    <vbox align="end">
> +      <button id="clearHistoryButton"
> +          class="accessory-button"

nit: indent should align with the id attribute.

Comment 6

[Johann Hofmann [:johannh]]

Comment on attachment 8952310 [details]
Bug 1436568 - Make "clear your recent history" a button instead of a link.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/221562/diff/1-2/

Comment 7

[Johann Hofmann [:johannh]]

Comment on attachment 8952310 [details]
Bug 1436568 - Make "clear your recent history" a button instead of a link.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/221562/diff/2-3/

Comment 8

[(inactive) Nihanth Subramanya [:nhnt11]]

Comment on attachment 8952310 [details]
Bug 1436568 - Make "clear your recent history" a button instead of a link.

https://reviewboard.mozilla.org/r/221562/#review228564

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ce6adae0faa1
Make "clear your recent history" a button instead of a link. r=nhnt11

Comment 10

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/ce6adae0faa1

Comment 11

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 60.0a1 (2018-02-07) on Windows 10, 64 Bit!

This bug's fix is verified with latest Nightly!

Build ID   : 20180225100219
User Agent : Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0

Comment 12

[roxleitan]

Verified fixed using Nightly 61.0a1(2018-03-13) and Beta 60.0b3 on Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.12.