403090 - [FIX]Found text is no longer highlighted within INPUT text fields and TEXTAREAs

Status: RESOLVED FIXED

(SeaMonkey :: Search, defect)

Description

[Darren]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en; rv:1.8.1.10pre) Gecko/20071108 Camino/1.5.4pre
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en; rv:1.8.1.10pre) Gecko/20071108 Camino/1.5.4pre

When searching for a string within a page that has form fields or textarea fields with text within the fields, Camino no longer highlights the found text string. Cursor appears to move, but visually, there's very little indication that the string has been found within the field.

Reproducible: Always

Steps to Reproduce:
1. Visit a page with a TEXTAREA or INPUT field with some text prefilled.
2. Search for a known string within the TEXTAREA or INPUT field.
3. Camino will fail to hilite the found text.
Actual Results:  
TEXTAREA scrolls and cursor is moved, but text string is not color highlighted

Expected Results:  
Text should be highlighted.

This feature was previously working around version 1.5.1.

Comment 1

[Chris Lawson (gone)]

This broke somewhere between 20071003 and 20071004, probably due to bug 388784.

Comment 2

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Confirming.

This broke between 2007-10-03 00:00 and 2007-10-04 01:00 Camino branch builds.

Checkins: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=MOZILLA_1_8_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-10-03+00%3A00&maxdate=2007-10-04+02%3A00&cvsroot=%2Fcvsroot

Lots of fun-looking bugs in there; bug 388784 and bug 393770 seem vaguely possible. If I knew anything about code, I'd lean toward Smaug's bug over Mats's bug.

I tried to see if this was broken in SeaMonkey 1.1.x also (since we're two apps that don't use Firefox TAF), but I can't get any SeaMonkey branch builds to run properly on 10.3.9 or 10.5 :/

Comment 3

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #2)
> I tried to see if this was broken in SeaMonkey 1.1.x also (since we're two apps
> that don't use Firefox TAF), but I can't get any SeaMonkey branch builds to run
> properly on 10.3.9 or 10.5 :/

Nevermind; apparently SM on the branch can't run correctly from the .dmg.  SeaMonkey's Find (Cmd-F) shows this lack-of-highlight bug as well.

Comment 4

[Samuel Sidler (old account; do not CC)]

This happens in SeaMonkey 1.1.6 on Windows as well.

Comment 5

[Boris Zbarsky [:bzbarsky]]

So this is not a problem in Firefox on branch?

Comment 6

[Samuel Sidler (old account; do not CC)]

No, it's only a problem for uses of the XPFE find-as-you-type implementation, I believe. I can't reproduce it in Firefox 2.0.0.9.

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

s/find-as-you-type/standard non-find-as-you-type find/

(On the branch, XPFE FAYT places the cursor but doesn't highlight; that's another bug that was filed long ago, stolen pre-Firefox 2 to use as a place to stash the Firefox fix for FAYT highlight, and closed, but it's not this bug.  This bug is about the standard Find impl that's not based on FAYT/TAF code.)

Comment 8

[Boris Zbarsky [:bzbarsky]]

Hmm.  Anyone happen to know where in the find code the highlighting's supposed to be happening?

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Does this happen with pages which don't have <label> connected to <input> or <textarea>? Or does XPFE n-FAYT-f use <label> for something?
Bug 388784 is only about focusing <label>.

Comment 10

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: <label>-less testcase

I can reproduce the failure using this very simple testcase :(

Comment 11

[Boris Zbarsky [:bzbarsky]]

This is absolutely a regression from bug 393770.  That patch changed what IsNativeAnonymous() returns for the textnodes inside inputs, and the find code depended on the old value.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

This seems to fix the problem for me.  On trunk, it looks like the use of nsIFrame::GetSelectionController is what saves the day... In fact, on trunk this code should just not walk out of anonymous content perhaps.  If we ever DO return true for IsNativeAnonymous on those textnodes on trunk, it will break.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Attachment: Trunk patch as described

Comment 14

[Daniel Veditz [:dveditz]]

1.8.1.10 is being rushed, this will have to catch the next train after some trunk baking of the patch

Comment 15

[Boris Zbarsky [:bzbarsky]]

Daniel, there isn't going to be any trunk baking.  This is a branch-only regression.  The trunk patch just adjusts the code to be a little less regression-prone in the future, but doesn't change any behavior.

It might still be good to get some branch baking, of course, so I agree that we'll have to make do with 1.8.1.11.

Comment 16

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

bz's patch does fix the issue for us in Camino (in case anyone was waiting for that info).

Comment 17

[Mats Palmgren (inactive)]

Comment on attachment 288113 [details] [diff] [review]
Fix

>-    if (!content->IsNativeAnonymous()) {
>+    if (!IsNativeAnonymous(content)) {

Maybe we should us a different name to reduce the risk of mistaking
one for the other? (IsNativeAnonymousDescendent(content) perhaps?)

Looks fine otherwise.  r+sr=mats

Comment 18

[Mats Palmgren (inactive)]

Comment on attachment 288114 [details] [diff] [review]
Trunk patch as described

ditto

Comment 19

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 288113 [details] [diff] [review]
Fix

Requesting branch approvals.  This is quite safe, but I would probably put it in .11 if .10 is anywhere close to being done.

Comment 20

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 288114 [details] [diff] [review]
Trunk patch as described

Requesting trunk approval.  This is bulletproofing/cleanup that should prevent this bug regressing on trunk if our anonymous content behavior is tweaked slightly.

Comment 21

[Boris Zbarsky [:bzbarsky]]

Checked in trunk patch, with s/IsNativeAnonymous/IsInNativeAnonymousSubtree/g.  Leaving bug open, since this is filed as a 1.8 branch bug.

It would be nice to have tests here; I just don't know our Find APIs well enough to tell which ones to use for said test.  Can someone help?

Comment 23

[Daniel Veditz [:dveditz]]

Not blocking because it won't stop us shipping Firefox, but will consider the patch for approval

Comment 24

[Daniel Veditz [:dveditz]]

Comment on attachment 288113 [details] [diff] [review]
Fix

approved for 1.8.1.12, a=dveditz for release-drivers

Comment 25

[Boris Zbarsky [:bzbarsky]]

Fixed on branch.

Comment 26

[Al Billings [:abillings - ex-MoCo]]

Sam, can you verify the fix using a Camino build, please?

Comment 27

[Samuel Sidler (old account; do not CC)]

Verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en; rv:1.8.1.12pre) Gecko/20080116 Camino/1.6b1 (like Firefox/2.0.0.12pre)

Comment 28

[Darren]

With ONE CAVEAT, this bug is FIXED in the latest nitely I have:
Version 2008011701 (1.5.5pre)

Command-F Find works as expected. Thanks!
/-key "Find as you type" does NOT hilite text.

Comment 29

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #28)
> With ONE CAVEAT, this bug is FIXED in the latest nitely I have:
> Version 2008011701 (1.5.5pre)
> 
> Command-F Find works as expected. Thanks!
> /-key "Find as you type" does NOT hilite text.

Darren, "find as you type" has never worked correctly; see comment 7.  This bug was explicitly about Command-F.

Setting this back to FIXED.

Comment 30

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

FWIW, I tracked down the bug I mentioned in comment 7 again.  It's bug 189039 and it stayed in Core:Keyboard:Find-as-you-type (the component for xpfe fayt), but the patch only touched toolkit/components/typeaheadfind/ (and some other toolkit code).

Comment 31

[Vicki Brown]

Confirmed in nightly. I just wanted to say THANK YOU.
(bug id=407432 was mine.  I NEED this functionality!)