142163 - Should be able to set UA stylesheet's ALINK (:active color colour pref)

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Håkan Waara]

Spin off from bug #109291.

One should be able to set the UA stylesheet's default active link color (ALINK)
from the UI.  For this to work, we also need to create a new function in
nsIPresContext; GetActiveLinkColor() or something like that.

This is required for document.body.aLink and document.alinkColor to work properly.

Comment 1

[Boris Zbarsky [:bzbarsky]]

.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

.

Comment 3

[Mats Palmgren (inactive)]

I feel brave today... patch coming up.

Comment 4

[Mats Palmgren (inactive)]

Attachment: Patch (base) rev .1

Comment 5

[Mats Palmgren (inactive)]

Attachment: Patch (editor) rev. 1

Comment 6

[Mats Palmgren (inactive)]

Attachment: Patch (embedding) rev. 1

Comment 7

[Mats Palmgren (inactive)]

I'm not sure who should review editor/embedding parts.
A note on the editor stuff:  I don't think the existing link/vlink colors is
working as it's supposed to, as I understand it the intention is that the
suggested page link colors (if the user has never set them through editor)
should be those from the Appearance/Color preferences (if set)?

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 132700 [details] [diff] [review]
Patch (base) rev .1

r=bzbarsky, though someone who does UI should OK the UI changes....

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 132701 [details] [diff] [review]
Patch (editor) rev. 1

sr=bzbarsky, I think glazou should be a good r= here..

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 132702 [details] [diff] [review]
Patch (embedding) rev. 1

sr=me, but Adam needs to review this.

Comment 11

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Comment on attachment 132701 [details] [diff] [review]
Patch (editor) rev. 1

sure
r=[e-mail]

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 132700 [details] [diff] [review]
Patch (base) rev .1

sr=dbaron, although you should get someone else to look at the xpfe part

Comment 13

[Adam Lock]

Comment on attachment 132702 [details] [diff] [review]
Patch (embedding) rev. 1

I don't understand this patch. I already have put/get alinkColor impls on
CIEHtmlDocument that correspond to the added lines in the patch so I'm not sure
what I'm meant to be reviewing.

Comment 14

[Mats Palmgren (inactive)]

Comment on attachment 132702 [details] [diff] [review]
Patch (embedding) rev. 1

Doh!  the alink stuff is indeed already in place here.
Sorry to waste your time.

Comment 15

[Mats Palmgren (inactive)]

Attachment: Patch2 (base) rev. 1

I actually forgot one file (nsPresShell.cpp) in the first patch.
Sorry to bother you guys again.

Comment 16

[Mats Palmgren (inactive)]

Who should I ask to review the pref. GUI bits?

Comment 17

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 132778 [details] [diff] [review]
Patch2 (base) rev. 1

r=bzbarsky, assuming we want to underline XLinks when the pref is set (I think
we do).

Comment 18

[Mats Palmgren (inactive)]

Attachment: Screenshot of color pref UI

Comment 19

[[e-mail]]

Just by eyeing the XUL I'd say the UI is good.

Comment 20

[Mats Palmgren (inactive)]

This can be checked in now, assuming Neil's OK on the UI above is enough.
(I still don't have CVS commit rights ;)

Comment 21

[Boris Zbarsky [:bzbarsky]]

Checked in (Mats, you should really ask for a CVS account), but this seems to
regress Tp by 0.5% or so (which could well be noise; it's only been two cycles).

Comment 22

[Boris Zbarsky [:bzbarsky]]

And one more cycle later, looks like it was indeed just noise.