56858 - RFE: Pref to make link traversal select target

Status: VERIFIED FIXED

(Core :: Layout, enhancement, P3)

Description

[Heikki Toivonen (remove -bugzilla when emailing directly)]

I want to have a pref that controls whether or not the target of link (once it
is traversed) is selected or not. Currently it is not.

I implemented this functionality into DocZilla, a Mozilla-based browser
(http://www.doczilla.com).

I think it makes it easier to spot what a link is really pointing to if the
target is highlighted somehow. CSS3 will most likely address the styling.
However, CSS will not *select* the target. Selection is handy because as soon as
you traverse the link you can copy the target to clipboard.

I know that perhaps the majority of old websites use empty A for anchor
locations so this pref/styling wouldn't show anything different than what we do
now. Newer websites, and people with SGML/XML background are more likely to use
the ID attribute to mark whole sections, which makes more sense anyway.

Comment 1

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: Implementation in PresShell::GoToAnchor()

Comment 2

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: Make it possible to select root

Comment 3

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: testcase HTML

Comment 4

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: testcase XML

Comment 5

[Heikki Toivonen (remove -bugzilla when emailing directly)]

This turned out to be trickier than I thought. The problematic case is the
selection of the document's root element.

The first patch does nothing if the anchor happens to be the root element. This
is because we try to get the content's parent, which does not exist. In this
patch we must have the parent to be able to make a selection.

The second patch should work fine even if the anchor is the root element.
Unfortunately this causes crash in nsContentSubtreeIterator::Init() because the
code does QI to content and uses this pointer without null-checks. It is null
because the document does not implement the nsIContent implementation.

After this experiment it is my understanding that it is impossible to select a
complete document. I opened bug 59785 for this.

It is possible to select all of the document EXCEPT the root element. In most
cases the user would think the whole document was selected. But if you were able
to paste HTML/XML or used code to check the contents of the selection you would
immediately notice it is wrong.

Comment 6

[Hixie (not reading bugmail)]

Upon managerial request, adding the "testcase" keyword to 84 open layout bugs that
do not have the "testcase" keyword and yet have an attachement with the word
"test" in the description field. Apologies for any mistakes.

Comment 7

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Anthony, could you review my second patch here?

How is it going with bug 59785, you said you had the fix in your tree a while
ago...?

Comment 8

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: Updated patch to work after layout split

Comment 9

[Heikki Toivonen (remove -bugzilla when emailing directly)]

This is weird... Applying the latest patch the HTML testcase works fine, and the
XML testcase doesn't crash either. The only problem is with the link to the
document root in the XML sample, which doesn't select anything (but it does not
crash either). Brief debugging shows that we get into GoToAnchor() twice which
seems absurd...

Comment 10

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Hmm, now I know why we can get into GoToAnchor() twice. If the first time
returns an error, we try again after intl conversion magic. Even though I don't
like dropping possibly serious error returns, maybe that is what is needed here
for the select code. After all, the caller of GoToAnchor() WILL drop return values.

Comment 11

[Chris Waterson]

so are you happy with the last patch? if so, sr=waterson

Comment 12

[vidur (gone)]

r=vidur for the last patch.

Comment 13

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Fix checked in.

Comment 14

[Chris Petersen]

xlink issue I believe

Comment 15

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Actually this applies to all links with #ref.

Comment 16

[bsharma]

Verified on
build: 2001-05-29-20-Trunk
platform: Win NT

The target are getting highlighted now in both XML and HTML test cases.