848455 - Comment anchor links do not scroll to the correct part of the page in the Mozilla theme

Status: RESOLVED FIXED

(bugzilla.mozilla.org Graveyard :: Sandstone/Mozilla Skin, defect)

Description

[(no longer active)]

See <https://bugzilla.mozilla.org/show_bug.cgi?id=476536#c8> for example.

Comment 1

[David Lawrence [:dkl]]

Not a regression as the Sandstone skin is new for BMO 4.2.

Comment 2

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Attachment: Add some invisible margin :before comment anchor elements

This seems to work for me. With this patch applied, clicking a link to a comment shows that comment's header in full below the fixed site header.

This invisible margin's height is hardcoded to 80px, which seems to work for me. Can the fixed site header be taller/shorter than ~80px? Is there a better way to adjust to the header's height than this?

Comment 3

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to Wes Kocher (:KWierso) from comment #2)
> Is there a better way to adjust to the header's height than this?

Also, should this apply to more than the bug comment anchors?

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Attachment: Add some invisible margin :before comment anchor elements v2

I must've uploaded an incomplete version of the patch last time. Fixed in this version, hopefully.

Comment 5

[David Lawrence [:dkl]]

Bram, since you are heavily into this part of BMO at the moment, can you take a look at Wes' changes and see if they look good and we can get the fixes committed?

Thanks
dkl

Comment 6

[Bram Pitoyo [:bram]]

kwierso’s CSS change looks good to me.

The only suggestion I would make is to shift the margin-top and height values by 15px (so they become -95px and 95px, respectively). This way, each comment gets a white space between it and the header.

Other than this, we’re good to go for committing the fix

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Attachment: bmocommentanchors.diff

Updated version with 95/-95 values.

I don't think I have commit access to BMO, so someone else would have to push it.

Comment 9

[David Lawrence [:dkl]]

I will check it in.

Comment 10

[David Lawrence [:dkl]]

Committing to: bzr+ssh://[e-mail]/bmo/4.2
modified skins/contrib/Mozilla/global.css
Committed revision 8883.

Comment 11

[David Lawrence [:dkl]]

There are quite a few other places that use named anchors that are affected by this and have the same issue as the comments of show_bug.cgi.

Glob and I discussed this and we will remove the fixed position setting for the header for now and address this in a different bug to make sure we find a solution that will work everywhere. We would like to get this skin as the default soon so we feel it is a good tradeoff for now.

Committing to: bzr+ssh://[e-mail]/bmo/4.2
modified skins/contrib/Mozilla/global.css
Committed revision 8948.

dkl