Bug 5393 – Anchor links do not jump to ID attributes on table row elements

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Andre-John Mas]

I have a page with an anchor named "row-21-15". If I enter the following URL,
which includes GET style parameters, the page does not jump to the section on
the page where the named achor is defiend:

http://localhost:8080/webapp/daybody.jsp?y=2005&M=10&d=18&ref=row-21-15#row-21-15

I tested this with webkit based browsers OmniWeb and Safari and both produce the
same incorrect results. On the other hand if I test it with Opera and Firefox,
both jump to the specifed anchor.

[Daniel Udey]

Do you have a specific URL that is available to the public where one might witness this bug? I have tried 
to reproduce it by adding GET strings to other URLs with anchor tags, and have not been able to 
reproduce it as of yet. If it is not possible to produce a publicly-accessible URL, is it possible to provide 
the HTML of your JSP page on which this occurs.

URLs I have tested:
http://httpd.apache.org/docs/2.0/mod/core.html?test=foo#enablemmap
http://sites.darien.ca/temp/web/anchor-webkit-test.html?testing=foo#anchor
http://sites.darien.ca/temp/web/anchor-webkit-test.html?
y=2005&M=10&d=18&ref=row-21-15#row-21-15

(In reply to comment #0)
> I have a page with an anchor named "row-21-15". If I enter the following URL,
> which includes GET style parameters, the page does not jump to the section on
> the page where the named achor is defiend:
> 
> http://localhost:8080/webapp/daybody.jsp?y=2005&M=10&d=18&ref=row-21-15#row-21-15
> 
> I tested this with webkit based browsers OmniWeb and Safari and both produce the
> same incorrect results. On the other hand if I test it with Opera and Firefox,
> both jump to the specifed anchor.

[Andre-John Mas]

Created attachment 4370 [details]
Failed anchor jump case

This page does not jump to the anchor named "row-21-15" if I type in the
follong URL:

http://localhost/daybody3.html?param=1234&param2=abcdef#row-21-15

it does work if I try it in Firefox. I just did a few changes to make sure that
it does validate properly with the markup validator, but that does not seem to
resolve the issue.

[Daniel Udey]

Created attachment 4372 [details]
Test case for misbehaving id on table row

Attached is a test case for this bug. The bug actually has nothing to do with
GET parameters; rather, it has to do with how Webkit handles anchors on table
row elements. The problem is that when WebKit jumps to an ID on a table row
element, it mistakenly moves the viewport to the top of the parent element,
instead of the top of the table row.

In HTML, a named anchor link (such as #row-21-15) can refer to two
possibilities: it can either refer to a named anchor (<a name="row-21-15">) or
to an ID (e.g. <h1 id="row-21-15">). The problem arises because WebKit does not
properly handle named anchors when the target is an ID of a table row. In the
test case, the <td id="row-21-15"> is the first applicable named anchor point,
so WebKit jumps to that, which takes it to the top of the parent element,
instead of jumping to the table row or the named anchor. This bug does not
occur if the ID is set on a <td>. Also, if the table row is a child of a
<tfoot> element, WebKit jumps to the <tfoot> element at the bottom of the
table.

A temporary workaround for web developers is to ensure that the ID of a table
row is not also the target of a named anchor link (for example, change <a
name="row-21-15"> to <a name="link-row-21-15">).

[Dave Hyatt]

Created attachment 7102 [details]
Patch that gives tables rows and sections correct dimensions.

This patch gives table rows and sections correct dimensions.

This creates interesting issues that had to be solved, namely that cells are not in row coordinate space, but are instead in section coordinate space.  This meant that various functions that naively crawl up the render tree to adjust coordinate spaces have to be corrected to not incorrectly add in row offsets when dealing with cells.

Some other things to note are that RenderTableRows and RenderTableSections cannot be hit tested.  This matches other browsers.  We had this right before, since these objects had no dimensions before (RenderTableSection had a height but a zero width), but now we have to be explicit about excluding them since they have valid bounding boxes.

Finally, in order to continue to deal with the insanity of borderTopExtra and borderBottomExtra, I've made whether or not to include borderTopExtra a required flag on absolutePosition.  This will force anyone who uses this function to think about what number it is they actually want.  This whole feature is insane and table cells should someday be reworked simply to have both outer and inner boxes instead of trying to do both using only one block.

[Dave Hyatt]

Created attachment 7106 [details]
Fix offsetHeight to include borderTop/BottomExtra.  Add temporary code to the render tree dumper to keep the results the same.

[Darin Adler]

Comment on attachment 7106 [details]
Fix offsetHeight to include borderTop/BottomExtra.  Add temporary code to the render tree dumper to keep the results the same.

Talking with Hyatt and Maciej on IRC, the 3 of us agreed the new bool parameter to absolutePosition is too confusing. Hyatt's mulling over some alternate approaches to that.

[Dave Hyatt]

Created attachment 7110 [details]
Rework things to make the internal absolutePosition recursion just always correct.   Add a new absolutePositionForContent method for the case where you want the inner box instead.

[Darin Adler]

Comment on attachment 7110 [details]
Rework things to make the internal absolutePosition recursion just always correct.   Add a new absolutePositionForContent method for the case where you want the inner box instead.

That one place in RenderObject::absoluteBoundingBoxRect looks like it should call absolutePositionForContent instead of adding in borderTopExtra.

There's still an issue with having to know when to use absolutePositionForContent, but I think this is way easier to understand than the extra bool was.

Looks good to me, r=me.

[Dave Hyatt]

Fixed.