500853 - "a" classes in Bugzilla CSS cause non-href anchors to style like links

Status: RESOLVED FIXED

(Bugzilla :: User Interface, defect)

Description

[Bradley Baetz (:bbaetz)]

terms.bug => terms.abug, and one missing </a> tag

Applies to 3.4 too

Comment 1

[Bradley Baetz (:bbaetz)]

Attachment: patch

Comment 2

[Max Kanat-Alexander]

Comment on attachment 385523 [details] [diff] [review]
patch

>Index: template/en/default/pages/fields.html.tmpl
>-<h2><a name="importance">Importance</h2>
>+<h2><a name="importance">Importance</a></h2>

  Because of the way our CSS is, "Importance" will get highlighted as a link there. (We need to have an a[name] rule that avoids that, but that's another issue.) It'd probably be better to do "<a></a>Importance" instead.

Comment 3

[Bradley Baetz (:bbaetz)]

(In reply to comment #2)
> (From update of attachment 385523 [details] [diff] [review])
> >Index: template/en/default/pages/fields.html.tmpl
> >-<h2><a name="importance">Importance</h2>
> >+<h2><a name="importance">Importance</a></h2>
> 
>   Because of the way our CSS is, "Importance" will get highlighted as a link
> there. (We need to have an a[name] rule that avoids that, but that's another
> issue.) It'd probably be better to do "<a></a>Importance" instead.

True, but all the others are like that....

Comment 4

[Frédéric Buclin]

Comment on attachment 385523 [details] [diff] [review]
patch

bbaetz is correct that all other H2 titles are already affected by this unwanted CSS rule.


>+    <td>The <b>status</b> field indicates the general health of 
>+    [% terms.abug %]. Only certain status transitions are allowed.</td>

You should write [%+ terms.abug %] as it's at the beginning of a line.


>+    You can enter a URL to [%+ terms.abug %] in the "Add [% terms.Bug %] URLs"

No need to put "+" here, [% terms.abug %] is in the middle of a line.


Could you update your patch with these two comments fixed + the missing rule about H2 titles?

Comment 5

[Bradley Baetz (:bbaetz)]

Attachment: v2

Fixing the <a name=> rule was a bit of a pain. the :link pseudoclass matches unvisited links, so we use :link:hover and :visited:hover (see https://developer.mozilla.org/en/Issues_Arising_From_Arbitrary-Element_hover)

The change to the Dusk color rule is because that has a higher specificity than :hover by itself, so without adding a:visited:hover to the rules the one in global.css would match. Googling suggests that this won't work in ie4, and ie5 will treat it as a:hover which may not work with some of the other rules. I don't think we care, though.... Anyone got ie6 to test?

Comment 6

[Frédéric Buclin]

We don't care about IE4 and IE5, and I have IE6 and IE8, so I can test.

Comment 7

[Bradley Baetz (:bbaetz)]

OK. You need to check (with both skins) that visited and unvisited links are underlined and have the correct colour when hovered over.

This is, btw, likely to break other skins since the standard global.css will now win.

Comment 8

[Max Kanat-Alexander]

Comment on attachment 385712 [details] [diff] [review]
v2

No, please don't use a:link. I suspect this doesn't fix everything, based on my experiments with a:link when I fixed a bunch of CSS before.

Instead, just have a rule for a[name] { text-decoration: inherit; color: inherit; background-color: inherit; }

Comment 9

[Bradley Baetz (:bbaetz)]

Do you know what specifically broke? Its following the advise from the mozilla wiki, and it works for me.

The problem with your suggestion is that it doesn't help against:

div.foo a { color: red }

to make all the links in given <div> red - the inherit rule has lower specificity.

(untested)

Comment 10

[Max Kanat-Alexander]

Okay, I'm starting to agree with you about a:link, we just have to be sure to use it everywhere. (Otherwise we have to have special rules for a[name][href] everywhere that we have a rule for a.) Could we split the CSS issues out into another bug? I think that will make the HTML changes r+.

Comment 11

[Frédéric Buclin]

So what's the status of this patch? It looks right to me.