129971 - Ctrl+F doesn't find "Read More" at slashdot

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Jesse Ruderman]

2002 031008 Win98

Steps to reproduce:
1. Load Slashdot in light mode (http://slashdot.org/index.pl?light=1).
2. Search for the text "read" or "of" or "comments".

Result: Mozilla never finds any text in "( Read More... | 67 of 187 comments )"
sections, but does find the text in other sections.

This happens with builds older than 3/10, so it's not a result of aaronl's fix
for bug 66597.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Jesse, is this the new find code or the old find code?  Or both?

Comment 2

[Paul Wyskoczka]

--> qa [e-mail]

Comment 3

[Jesse Ruderman]

bz, I'm not sure what bug/patch you're referring to, but I tested a build from
January and that build didn't have this bug.

1/27 works fine
3/02 broken
3/10 broken

Comment 4

[Akkana Peck]

I see it, too, with the new find code.

The segments in question look like this:
<!-- no ad 6 --><p><b>(</b> <a
HREF="//slashdot.org/article.pl?sid=02/03/10/2134213&amp;mode=thread&amp;tid=95"><b>Read
More...</b></a> | <b><a
HREF="//slashdot.org/article.pl?sid=02/03/10/2134213&amp;mode=thread&amp;tid=95&amp;threshold=1">29</a>
of <a
HREF="//slashdot.org/article.pl?sid=02/03/10/2134213&amp;mode=thread&amp;tid=95&amp;threshold=-1">49</a></b>
comments <b>)</b></p>

I don't see anything obvious that explains why they're not being found.  Will
investigate.

Comment 5

[Akkana Peck]

Whoops, I marked accepted and didn't notice it wasn't assigned to me.  It should
be, so I'm taking it.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Jesse, fyi the bugs I meant were bug 97157, bug 123087, etc (the big find
rewrite Akkana did).

Comment 7

[Akkana Peck]

*** Bug 130890 has been marked as a duplicate of this bug. ***

Comment 8

[Akkana Peck]

Here's the deal: This and bug 130890 are happening because we're skipping a few
nodes after we encounter a comment.  The missed matches are all right after a
comment node, and if you remove the comment, the strings are found.

Setting appropriate keywords and TFM.

Comment 9

[Akkana Peck]

There was a while loop in the code which was skipping to the next sibling when
it hit a skipped node, except that was skipping too far and skipping right over
content it needed to include.

That code was actually written for an iterator with different behavior than the
one we have, which is still an RFE.  The way the code currently works, we don't
need any of that node-skipping while loop, because we're already inside a loop
checking SkipNode, and Skip node has since been rewritten to climb up the chain
and compare node parents.  So all we really need to do here is: if
(SkipNode(content)) continue.  This fixes the problem on both sample pages.

Seeking review for the patch I'm about to attach.

Comment 10

[Akkana Peck]

Attachment: Fix: don't skip so many nodes

Here's the patch.  It passes all the find regression tests.  I'll be adding a
few cases to the regression tests to cover this case, so that the trunk won't
pass it but will after the patch.

Comment 11

[Akkana Peck]

This patch has been rolled into the one in bug 80805, which is awaiting checkin
approval.  Marking dependency.

Comment 12

[Akkana Peck]

The fix is in; today's slashdot should find those strings.

Comment 13

[Akkana Peck]

*** Bug 132612 has been marked as a duplicate of this bug. ***

Comment 14

[Patty Mac]

Verified on all platforms (commercial netscape build: 2002-03-22-09-TRUNK).