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

RESOLVED FIXED

Status

()

bugzilla.mozilla.org
Sandstone/Mozilla Skin
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Away for a while, Assigned: KWierso)

Tracking

Production

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

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

Updated

5 years ago
Component: User Interface → Sandstone
OS: Mac OS X → All
Hardware: x86 → All
Not a regression as the Sandstone skin is new for BMO 4.2.
No longer blocks: 848218
(Assignee)

Comment 2

5 years ago
Created attachment 736187 [details] [diff] [review]
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?
Assignee: nobody → kwierso
Status: NEW → ASSIGNED
Attachment #736187 - Flags: feedback?
(Assignee)

Comment 3

5 years ago
(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?
(Assignee)

Comment 4

5 years ago
Created attachment 737201 [details] [diff] [review]
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.
Attachment #736187 - Attachment is obsolete: true
Attachment #736187 - Flags: feedback?
Attachment #737201 - Flags: feedback?(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
Flags: needinfo?(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
Flags: needinfo?(bram)

Updated

5 years ago
Duplicate of this bug: 894989
(Assignee)

Comment 8

5 years ago
Created attachment 777656 [details] [diff] [review]
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.
Attachment #737201 - Attachment is obsolete: true
Attachment #737201 - Flags: feedback?(dkl)
Flags: needinfo?(bram)
I will check it in.
Flags: needinfo?(bram)
Committing to: bzr+ssh://dlawrence%40mozilla.com@bzr.mozilla.org/bmo/4.2
modified skins/contrib/Mozilla/global.css
Committed revision 8883.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

4 years ago
Depends on: 898999
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://dlawrence%40mozilla.com@bzr.mozilla.org/bmo/4.2
modified skins/contrib/Mozilla/global.css
Committed revision 8948.

dkl

Updated

4 years ago
Blocks: 905763

Updated

4 years ago
No longer blocks: 905763
You need to log in before you can comment on or make changes to this bug.