Closed
Bug 697750
Opened 13 years ago
Closed 13 years ago
Follow-up: replace about:home sync "links" (actually buttons) with links.
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 10
People
(Reporter: liuche, Assigned: liuche)
References
Details
(Whiteboard: [verified in services])
Attachments
(1 file, 3 obsolete files)
|
2.75 KB,
patch
|
Gavin
:
review+
|
Details | Diff | Splinter Review |
Currently sync links are actually buttons, to conform with browser.js click listener code. Replace buttons with actual links and propagate to browser.js.
|
Assignee | |
Comment 1•13 years ago
|
||
verified visually through UI and ran tests from bug 675821.
Attachment #570074 -
Flags: review?(philipp)
|
||
Comment 2•13 years ago
|
||
Comment on attachment 570074 [details] [diff] [review] v1: replaced buttons with links Looks ok to me. Over to Gavin!
Attachment #570074 -
Flags: review?(philipp) → review?(gavin.sharp)
|
||
Comment 3•13 years ago
|
||
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.
Attachment #570074 -
Flags: review?(gavin.sharp) → review+
|
|
Assignee | |
Comment 4•13 years ago
|
||
Underline does seem unnecessary, but the cursor doesn't appear for me without explicitly adding it in the css, not sure why.
Attachment #570074 -
Attachment is obsolete: true
Attachment #570189 -
Flags: review?(gavin.sharp)
|
|
||
Comment 5•13 years ago
|
||
Comment on attachment 570189 [details] [diff] [review] v2: removed underline css Does the hover stuff work if you make the links have href="#"?
|
|
Assignee | |
Comment 6•13 years ago
|
||
No, href="#" still lacks hover cursor.
|
|
||
Comment 7•13 years ago
|
||
This seems to work for me, does it seem OK to you?
Attachment #570399 -
Flags: feedback?(liuche)
|
|
Assignee | |
Comment 8•13 years ago
|
||
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.
Attachment #570399 -
Flags: feedback?(liuche) → feedback+
|
|
Assignee | |
Comment 9•13 years ago
|
||
|
|
||
Updated•13 years ago
|
Attachment #570189 -
Attachment is obsolete: true
Attachment #570189 -
Flags: review?(gavin.sharp)
|
|
||
Updated•13 years ago
|
Attachment #570755 -
Flags: review+
|
|
Assignee | |
Updated•13 years ago
|
Attachment #570399 -
Attachment is obsolete: true
|
|
||
Comment 10•13 years ago
|
||
https://hg.mozilla.org/services/services-central/rev/9215e529ae7a
Status: NEW → ASSIGNED
Whiteboard: [fixed in services]
|
|
||
Updated•13 years ago
|
|
||
Updated•13 years ago
|
Assignee: nobody → liuche
Component: Firefox Sync: UI → General
Product: Mozilla Services → Firefox
QA Contact: sync-ui → general
|
||
Comment 11•13 years ago
|
||
verified with latest s-c build
Whiteboard: [fixed in services] → [verified in services]
|
|
||
Comment 12•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9215e529ae7a
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 10
You need to log in
before you can comment on or make changes to this bug.
Description
•