Closed Bug 412646 Opened 14 years ago Closed 14 years ago

Dot under caret in find bar

Categories

(Core :: Layout, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9

People

(Reporter: ria.klaassen, Assigned: martijn.martijn)

References

(Depends on 1 open bug)

Details

(Keywords: polish, regression)

Attachments

(10 files, 1 obsolete file)

Between two blinks of the caret I see a dot in the find bar's input field.

When I remove the code 
#FindToolbar {
 overflow-x: hidden;
the problem disappears.

I first noticed the problem because in spite of the userChrome.css code
#FindToolbar { -moz-box-direction: reverse; }
the find bar appeared on an unusual place.
Component: Find Toolbar / FastFind → Layout
Product: Firefox → Core
QA Contact: fast.find → layout
Attached image screen shot
No Dot: 20080115_1726_firefox-3.0b3pre.en-US.win32
Dot: 20080115_1957_firefox-3.0b3pre.en-US.win32

Checkins to module PhoenixTinderbox between 2008-01-15 17:26 and 2008-01-15 19:56 : 
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1200446760&maxdate=1200455819
Flags: blocking1.9?
I see this on Linux, too.
OS: Windows XP → All
Hardware: PC → All
The dot appeared for the first time in June 2006; it was a 1 pixel dot.
It became a two pixels dot in Feb 2007 when bug 177805 was checked in.
Comment 4 was tested including overflow-x: hidden. If that code was used it would have appeared for the first time in June 2006. Don't know if it is useful to know when exactly.
In case there's no obvious fix, we should back out the find bar part of bug 312247.
(copyed from Bug 311247 Comment 38)
I think dot in find bar text box is caused by incorrect height of blinking
caret.
When caret is drawn by reflow, window expose, moving caret by text input, its
height is correct.
But when automatic blinking, its height is not valid (top is valid. But bottom
is not valid).
Blocks: pixels, 287813
Both problems from comment 0 can temporarily be corrected with a userChrome.css with 

#FindToolbar {
  overflow-x: visible !important;
}

Is problem 2 a separate bug?

Attached file testcase
You should see some text appear in the text field.
Duplicate of this bug: 413244
Flags: wanted1.9+
Flags: blocking1.9?
Flags: blocking1.9-
Duplicate of this bug: 413940
Note that this also seems to affect the "close" button at the left end on the find bar -- when I mouseover it, its lower border is missing.
  (unless the cursor happens to be hovering over it when the find-bar appears, in which case the lower border sticks around when it shouldn't be there.)
Martijn's workaround can fix Bug 414708 too.

Note that with Martijn's workaround, the overflow-x: hidden; declaration can be removed. It seems that that declaration is only necessary because of the align="center" part on the findbar.
Oh, that is useful information. Thanks, Teune.
In that case, when that overflow-x: hidden; rule can be removed, then my "workaround for the issue" patch might even be preferred. (although that doesn't mean this should be happening at all)

This might be related to bug 412646, perhaps.
Duplicate of this bug: 416135
(In reply to comment #16)
> This might be related to bug 412646, perhaps.

I think you mean bug 413027.   (this bug here is bug 412646 :))
(In reply to comment #18)
> I think you mean bug 413027.   (this bug here is bug 412646 :))

Oops, yes, that's what I meant.

This is a testcase that tests performance of overflow: auto boxes.

trunk:
scrolltest: 5904ms
toggledisplaytest: 228913ms

Just for comparison, I also tested branch
branch: 
scrolltest: 7047ms
toggledisplaytest: 267140ms
This testcase uses two boxes instead of an overflow: auto box, it seems much faster.

trunk:
scrolltest: 4055ms
toggledisplaytest: 6922ms

branch:
scrolltest: 4218ms
toggledisplaytest: 8344ms
Attached patch patch (obsolete) — Splinter Review
This patch doesn't seem to regress bug 312247.
The pinstripe changes look strange. It seems to me those css rules could be made simpler, but I'm afraid I can't really test those pinstripe changes at all.
So anyone who could help with that, would be appreciated.
Attachment #302817 - Flags: review?(dao)
Comment on attachment 302817 [details] [diff] [review]
patch

toolkit/themes/winstripe/global/findBar.css
> }
>-
> /* find-next button */
> 
> .findbar-find-next {

That line shouldn't be removed.

toolkit/themes/pinstripe/global/findBar.css
>-findbar > toolbarbutton:not([disabled]):hover:active,
>-findbar > hbox > toolbarbutton:not([disabled]):hover:active {
>+findbar > hbox > toolbarbutton:not([disabled]):hover:active,
>+findbar > hbox > hbox > toolbarbutton:not([disabled]):hover:active {
>   background-image: url("chrome://global/skin/icons/white-gray-gradient-active.gif");
> }

Please add a class name to the new hbox, and use the class name of the existing hbox (find-field-container). After that, you can drop "findbar >" here and elsewhere.

Looks good otherwise -- please request review from a toolkit & browser peer.
Attachment #302817 - Flags: review?(dao) → review+
Assignee: nobody → martijn.martijn
Blocks: 417091
Duplicate of this bug: 417202
3.0b3.  regression, an H2 is not displayed correctly inside a marquee.
Blocks: 417638
Duplicate of this bug: 417933
Duplicate of this bug: 418151
Whiteboard: [needs new patch]
Isn't this serious enough to constitute a blocker for 1.9?
Flags: blocking1.9?
Duplicate of this bug: 417638
Duplicate of this bug: 423786
Given the number of dups let's get this fixed...
Flags: blocking1.9? → blocking1.9+
Priority: -- → P1
--> P2, not blocking beta 5 on this, will block final.
Priority: P1 → P2
Target Milestone: --- → mozilla1.9
Martijn, is this actually a duplicate, or at least dependent on, bug 413027?
Yes, I think this is basically a duplicate of bug 413027. However, this can be fixed by a workaround (the patch I attached, which is probably a better way of doing anyway).
Sorry for the delay here, I'll update the patch later today, hopefully.
Depends on: 413027
This could be just my build but at least for me the findbar.xml change breaks the 'find as you type' feature. With the patch starting typing no longer opens the quick find bar.
Attached patch patchv2Splinter Review
Sorry for the delay
Attachment #302817 - Attachment is obsolete: true
Attachment #312096 - Flags: review?(gavin.sharp)
Blocks: 426012
Comment on attachment 312096 [details] [diff] [review]
patchv2

This looks fine, but need some small changes:
- no need to move the "findbar" rules to ".findbar-container"
- need to update the "findbar[flash="true"]  > .find-field-container" selector to take into account the new hbox, for both winstripe and gnomestripe
Attachment #312096 - Flags: review?(gavin.sharp) → review-
Duplicate of this bug: 426775
Duplicate of this bug: 426781
Duplicate of this bug: 427388
Martijn you have an updated patch here?
This is pretty minor visual artifact - I think at this point we should move this bug to .next... 
Flags: wanted-next+
Flags: blocking1.9-
Flags: blocking1.9+
Flags: wanted1.9.0.x?
This might only be a dot in Windows. But on Mac OS X you actually see a very fat black underline on the text you typed in.

This fat black underline makes it difficult to see what you just typed.

This is a highly visible regression that already have eleven duplicates. Please reconsider the blocking status.
Same under Linux, at least for me.
"This is pretty minor visual artifact" - maybe for users that don't search much. But for me, doing searches practically on every second visited page, this is quite serious. FF3, which looks otherwise as a very polished product is now viewed as this old unbreakable "cheapo FOSS product" myth (I haven't seen such behavior in any other SW product I regularly use, be it FOSS or commercial).

Moreover, this is a regression from previous FF versions, so it should be high on the list. If it were there from the beginning I would get used to it, but this way it sheds some bad light on FF3...

So please reconsider. And sorry for spamming this bug.
I really hate to add a "me too", but even after many weeks of getting used to this bug, it still catches me out. I keep thinking I've accidentally typed a period in the search and still often habitually backspace to try to get rid of it, before I remember.
Would like drivers to reconsider, as we have an almost-finished patch for an issue that while not being harmful to the user is a very bad user experience for a feature that is very widely and frequently used.
Flags: blocking1.9- → blocking1.9?
Keywords: polish
Sorry, but for me it's not only a minor visual artifact, but the complete lower part of some letters is missing: the letter 'y' cannot be distinguished from the letter 'v', 'g' looks like 'o', and '_' (underline) is not displayed at all.

I filed a separate bug 426775 for this, but it was marked as a duplicate of this one.
OK, blocking.  Going to finish this Martijn?  Need a reviewed patch, ASAP if we're going to get this in.  I reserve the right to change my mind.  :)

Flags: blocking1.9? → blocking1.9+
Attached patch patchv3Splinter Review
New patch based on Martijn's, incorporating Gavin's comments.
* Moved ".findbar-container" rules back to "findbar"
* Fixed the flash=true selector for gnomestrip, pinstripe, and winstripe

This is a pretty annoying issue (under certain circumstances, it's not just a dot), so I hope this doesn't get pushed back to 3.0.1
Attachment #314477 - Flags: review?(gavin.sharp)
Comment on attachment 314477 [details] [diff] [review]
patchv3

Thanks!
Attachment #314477 - Flags: review?(gavin.sharp) → review+
Keywords: checkin-needed
Whiteboard: [needs new patch]
Comment on attachment 314477 [details] [diff] [review]
patchv3

a=mconnor on behalf of 1.9 drivers
Attachment #314477 - Flags: approval1.9? → approval1.9+
mozilla/browser/base/content/browser.css 	1.61
mozilla/toolkit/content/widgets/findbar.xml 	1.22
mozilla/toolkit/themes/gnomestripe/global/findBar.css 	1.4
mozilla/toolkit/themes/pinstripe/global/findBar.css 	1.11
mozilla/toolkit/themes/winstripe/global/findBar.css 	1.9

Thanks Martijn and Kai!
Status: NEW → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Attached patch bustage fixSplinter Review
I landed this to fix the test_bug288254.xul failure.
Attachment #314734 - Flags: review?(mano)
This has broken 'Quick Find' ..  the / or the ' does not bring up Find Bar, you see a slight movement at the bottom of the browser window, but the Bar is not displayed.  Also I see nothing in the Error Console

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008040915 Minefield/3.0pre Firefox/3.0 ID:2008040915 <-Latest hourly

(In reply to comment #55)
> This has broken 'Quick Find' ..  the / or the ' does not bring up Find Bar, you
> see a slight movement at the bottom of the browser window, but the Bar is not
> displayed.  Also I see nothing in the Error Console

That's the bustage that the bustage fix fixes :)
verified fixed using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008041005 Minefield/3.0pre. I verified by testing with the find bar and comparing it the build from yesterday.
Status: RESOLVED → VERIFIED
Depends on: 428351
(In reply to comment #56)
> (In reply to comment #55)
> > This has broken 'Quick Find' ..  the / or the ' does not bring up Find Bar, you
> > see a slight movement at the bottom of the browser window, but the Bar is not
> > displayed.  Also I see nothing in the Error Console
> 
> That's the bustage that the bustage fix fixes :)
> 
I filed bug 428351 for that
i have
#FindToolbar>* {display: -moz-box !important;}
in my userchrome.css to invoke full featured findbar when doing FAYT, now its not working anymore,  is it related to this patch?
.findbar-container>* {display: -moz-box !important;}
seems to work.
(In reply to comment #60)
> .findbar-container>* {display: -moz-box !important;}
> seems to work.
> 

thanks!
Duplicate of this bug: 428652
Duplicate of this bug: 426012
Duplicate of this bug: 420229
Duplicate of this bug: 420847
Duplicate of this bug: 433940
Depends on: 435231
No longer depends on: 435231
Flags: wanted1.9.0.x?
The core cause of this issue is still around. But I think that is basically bug 413027.
You need to log in before you can comment on or make changes to this bug.