Last Comment Bug 697750 - Follow-up: replace about:home sync "links" (actually buttons) with links.
: Follow-up: replace about:home sync "links" (actually buttons) with links.
Status: RESOLVED FIXED
[verified in services]
:
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Firefox 10
Assigned To: Chenxia Liu [:liuche]
:
:
Mentors:
Depends on:
Blocks: 675821
  Show dependency treegraph
 
Reported: 2011-10-27 10:31 PDT by Chenxia Liu [:liuche]
Modified: 2011-11-02 15:11 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1: replaced buttons with links (2.62 KB, patch)
2011-10-27 13:52 PDT, Chenxia Liu [:liuche]
gavin.sharp: review+
Details | Diff | Splinter Review
v2: removed underline css (2.62 KB, patch)
2011-10-27 22:30 PDT, Chenxia Liu [:liuche]
no flags Details | Diff | Splinter Review
tweaked patch (2.75 KB, patch)
2011-10-28 15:51 PDT, :Gavin Sharp [email: gavin@gavinsharp.com]
liuche: feedback+
Details | Diff | Splinter Review
v3: tweaked patch + compat recently landed localization changes (2.75 KB, patch)
2011-10-31 10:37 PDT, Chenxia Liu [:liuche]
gavin.sharp: review+
Details | Diff | Splinter Review

Description Chenxia Liu [:liuche] 2011-10-27 10:31:39 PDT
Currently sync links are actually buttons, to conform with browser.js click listener code. Replace buttons with actual links and propagate to browser.js.
Comment 1 Chenxia Liu [:liuche] 2011-10-27 13:52:13 PDT
Created attachment 570074 [details] [diff] [review]
v1: replaced buttons with links

verified visually through UI and ran tests from bug 675821.
Comment 2 Philipp von Weitershausen [:philikon] 2011-10-27 17:19:30 PDT
Comment on attachment 570074 [details] [diff] [review]
v1: replaced buttons with links

Looks ok to me. Over to Gavin!
Comment 3 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-10-27 18:02:07 PDT
Comment on attachment 570074 [details] [diff] [review]
v1: replaced buttons with links

Do you really need to override the default <a> styling at all? I could see perhaps the color change being desirable, but the :hover text-decoration and cursor rules seem like they should be unnecessary. r=me with that addressed.
Comment 4 Chenxia Liu [:liuche] 2011-10-27 22:30:14 PDT
Created attachment 570189 [details] [diff] [review]
v2: removed underline css

Underline does seem unnecessary, but the cursor doesn't appear for me without explicitly adding it in the css, not sure why.
Comment 5 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-10-28 12:48:53 PDT
Comment on attachment 570189 [details] [diff] [review]
v2: removed underline css

Does the hover stuff work if you make the links have href="#"?
Comment 6 Chenxia Liu [:liuche] 2011-10-28 14:28:31 PDT
No, href="#" still lacks hover cursor.
Comment 7 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-10-28 15:51:17 PDT
Created attachment 570399 [details] [diff] [review]
tweaked patch

This seems to work for me, does it seem OK to you?
Comment 8 Chenxia Liu [:liuche] 2011-10-31 10:35:34 PDT
Comment on attachment 570399 [details] [diff] [review]
tweaked patch

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

Yup, tried this out and it works for me as well. Adding patch that includes localization changes.
Comment 9 Chenxia Liu [:liuche] 2011-10-31 10:37:32 PDT
Created attachment 570755 [details] [diff] [review]
v3: tweaked patch + compat recently landed localization changes
Comment 10 Philipp von Weitershausen [:philikon] 2011-11-01 12:15:37 PDT
https://hg.mozilla.org/services/services-central/rev/9215e529ae7a
Comment 11 Tracy Walker [:tracy] 2011-11-02 14:48:20 PDT
verified with latest s-c build
Comment 12 Philipp von Weitershausen [:philikon] 2011-11-02 15:11:36 PDT
https://hg.mozilla.org/mozilla-central/rev/9215e529ae7a

Note You need to log in before you can comment on or make changes to this bug.