ltr caret in input field unexpected (directional caret appears when only ltr input methods are installed)

VERIFIED FIXED

Status

()

defect
VERIFIED FIXED
14 years ago
11 years ago

People

(Reporter: santiago.matamoros, Assigned: smontagu)

Tracking

(Blocks 1 bug, {testcase})

Trunk
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8.1 -

Firefox Tracking Flags

(Not tracked)

Details

()

Attachments

(5 attachments, 1 obsolete attachment)

Reporter

Description

14 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de-DE; rv:1.7.6) Gecko/20050223 Firefox/1.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de-DE; rv:1.7.6) Gecko/20050223 Firefox/1.0.1

In the german wikipedia the cursor in the input field looks strange.
The cursor seems to have a little dot on the left side, almost looking like a cross.
It occours at least in all versions of mozilla firefox from 0.5 to 1.0.1.
I was not able to reproduce it in IE or Opera. I never saw this on any other
site (including Wikipedia in other languages). 

The occurence of the effect seems to depend on the size of the html file.

See also
http://de.wikipedia.org/wiki/Wikipedia:Beobachtete_Fehler#Komischer_Cursor_in_Firefox
for a picture of the effect and a description in German.

Reproducible: Always

Steps to Reproduce:
1. Go to http://de.wikipedia.org/wiki/Hauptseite
2. Look at the input box on the left side.
3.

Actual Results:  
The cursor in the input box has a little dot on the left side, almost looking
like a cross.

Expected Results:  
Show the cursor as straight vertical bar.

It seems to depend on the size of the html page (not the css).
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050402
Firefox/1.0+
I'm seeing the same in the firefox URL bar & search bar.
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050402
Firefox/1.0+
But not with a new profile..

Comment 3

14 years ago
I'm seeing the little dot too, both in the German and the English Wikipedia. I'm
not seeing it anywhere else.

Comment 4

14 years ago
Moving to Core->Layout: Form Controls
Component: General → Layout: Form Controls
Product: Firefox → Core
Version: unspecified → Trunk

Comment 5

14 years ago
The caret (cursor) you are seeing is to indicate the you are in left to right.
mode. https://bugzilla.mozilla.org/attachment.cgi?id=167412 shows what you would
see if you are in right to left mode.

I'm not sure when, and when not these directional carets are ment to be shown,
and why it's showing on wikipedia. (Maybe because they specify
margin-left-ltr-source, margin-left-rtl-source, margin-right-ltr-source, and
margin-right-rtl-source)

Change Component to BiDi, Block 265070, updated summary
 
Blocks: BidiCaret
Component: Layout: Form Controls → Layout: BiDi Hebrew & Arabic
Summary: Stange cursor in input field → ltr caret in input field unexpected

Updated

14 years ago
Assignee: firefox → mkaply
QA Contact: general → zach

Comment 7

14 years ago
Posted file minimized testcase
If I remove the hebrew text, the cursor remains normal.

  <div>
	  <div>
	    <form name="searchform" action="" id="searchform">
	      <input id="searchInput" name="search" accesskey="f" value=""
type="text">
	    </form>
	  </div>
	  
	  <div>
	    <ul>
	      <li><a href="http://he.wikipedia.org/wiki/">עברית</a></li>
	      <li><a href="http://hr.wikipedia.org/wiki/">Hrvatski</a></li>
	    </ul>
	  </div>
 </div>

Comment 8

14 years ago
I didn´t check for other LTR languages, I just found that excluding the language
box brought back the cursor to normal, and then I started commenting half of the
<li>s, until I reduced it to two languages, one causing the bug, and the next
one, normal.
First testcase is original code from Wikipedia, second testcase contains no CSS.
Keywords: testcase

Comment 9

14 years ago
previous testcase uses multiple divs, this one has only one:

 <div>
  <input id="searchInput" name="search" accesskey="f" value="" type="text">
  <a href="http://als.wikipedia.org/wiki/">Alemannisch</a>
  <a
href="http://ar.wikipedia.org/wiki/">&#1575;&#1604;&#1593;&#1585;&#1576;&#1610;&#1577;</a>

 </div>
Assignee

Comment 10

14 years ago
Mano, did we discuss somewhere never showing the directional caret if there is
no RTL keyboard installed?
Not in particular, but we did agree we should have a "IsBiDiCaret" or such in
nsIBidiKeyboard in order to fix bug 272096, I guess an installed rtl keyboard
could be another condition.

Comment 12

14 years ago
Mano/smontagu: So, like, are you confirming this bug, marking it as a dupe or what?
Assignee

Updated

14 years ago
Depends on: 272096

Comment 13

14 years ago
Confirming bug. Users who don't have an RTL input method installed shouldn't
encounter the RTL caret. On any platform.

The other side of the coin is that BiDi users should never encounter a
non-directional caret. Simon, should this be a new bug?

Prog.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Hardware: PC → All
Summary: ltr caret in input field unexpected → ltr caret in input field unexpected (directional caret appears when only ltr input methods are installed)
Assignee

Comment 14

14 years ago
(In reply to comment #13)

> The other side of the coin is that BiDi users should never encounter a
> non-directional caret. Simon, should this be a new bug?

What do you mean by "Bidi users" in this context? Users with a Bidi
localization? Users with a Bidi locale? Users with an RTL input method installed?

Comment 15

14 years ago
By "BiDi users" (in this context), I mean "Users with an RTL input method
installed". 

To make the interface as consistent and as expected as possible, the types of
carets should be kept to the minimum that's essential. Currently, non-BiDi users
sometimes see directional carets which they don't understand, and BiDi users
sometimes see directionless caret which is inconsistent ("why is it LTR on this
page and directionless on that page?").

Prog.
Assignee

Comment 16

14 years ago
*** Bug 294419 has been marked as a duplicate of this bug. ***

Comment 17

14 years ago
*** Bug 301654 has been marked as a duplicate of this bug. ***
*** Bug 305429 has been marked as a duplicate of this bug. ***

Comment 19

13 years ago
*** Bug 343590 has been marked as a duplicate of this bug. ***

Comment 20

13 years ago
Site does not have to be in a foreign language for the cursor artifact to appear.

Check http://www.adobe.com/ and http://en.wikipedia.org/wiki/Main_Page
Assignee

Comment 21

13 years ago
(In reply to comment #20)
> Site does not have to be in a foreign language for the cursor artifact to
> appear.
> 
> Check http://www.adobe.com/ and http://en.wikipedia.org/wiki/Main_Page

I'm not sure what "in a foreign language" means in this context. The bidirectional cursor appears in documents that contain either intrinsic right-to-left content (e.g. Arabic or Hebrew) or elements with right-to-left directionality (e.g. with direction: rtl in CSS).
http://en.wikipedia.org/wiki/Main_Page contains links to wikipedias in several dozen different languages, several of which are right-to-left. 
http://www.adobe.com has an element with direction: rtl (it's empty and I don't know why it's there, but it is).
Assignee

Comment 22

13 years ago
*** Bug 343556 has been marked as a duplicate of this bug. ***
Assignee

Comment 23

13 years ago
Posted patch Windows patch (obsolete) — Splinter Review
Assignee: mozilla → smontagu
Status: NEW → ASSIGNED
Attachment #228843 - Flags: superreview?(roc)
Attachment #228843 - Flags: review?(roc)
   PRBool mDefaultsSet;
+  PRBool mHaveBidiKeyboards;

Make these PRPackedBool

+  // If there is not at least one keyboard of each directionality, return
+  // without setting any values. Bidi keyboard functionality will be disabled.
+  mHaveBidiKeyboards = (isRTLKeyboardSet && isLTRKeyboardSet);
+  if (!mHaveBidiKeyboards)
+    return NS_OK;

Why are we disabling Bidi keyboard functionality if there's only an RTL keyboard? Actually I'm not sure what it means for there to be only an RTL keyboard, maybe I'm not the best reviewer for this code.

+  if (!mDefaultsSet) {
+    nsresult result = EnumerateKeyboards();
+    if (NS_SUCCEEDED(result))
+      mDefaultsSet = PR_TRUE;
+    else
+      return result;
+  }
+  if (!mHaveBidiKeyboards)
+    return NS_ERROR_FAILURE;
+

Move this code into EnumerateKeyboards instead of duplicating it.
Assignee

Comment 25

13 years ago
(In reply to comment #24)
> Why are we disabling Bidi keyboard functionality if there's only an RTL
> keyboard? Actually I'm not sure what it means for there to be only an RTL
> keyboard, maybe I'm not the best reviewer for this code.

In theory I could have e.g. only Arabic Persian and Urdu keyboards installed and I would never enter left-to-right text. In practice I doubt that this ever occurs, but I thought it made more sense to cover the case.

> +  if (!mDefaultsSet) {
> +    nsresult result = EnumerateKeyboards();
> +    if (NS_SUCCEEDED(result))
> +      mDefaultsSet = PR_TRUE;
> +    else
> +      return result;
> +  }
> +  if (!mHaveBidiKeyboards)
> +    return NS_ERROR_FAILURE;
> +
> 
> Move this code into EnumerateKeyboards instead of duplicating it.
 
Umm, apart from setting mDefaultsSet, what can I move into EnumerateKeyboards? I still have to test mHaveBidiKeyboards even when mDefaultsSet is true.
Assignee

Comment 26

13 years ago
Oops, forgot to cc roc before responding to review comments
In EnumerateKeyboards, which you might want to call something else, you could have

  if (mDefaultsSet) {
    return mHaveBidiKeyboards ? NS_OK : NS_ERROR_FAILURE;
  ...
  mDefaultsSet = PR_TRUE;
  return mHaveBidiKeyboards ? NS_OK : NS_ERROR_FAILURE;  

then at the callers you just need
  nsresult rv = EnumerateKeyboards();
  if (NS_FAILED(rv))
    return rv;
Assignee

Comment 28

13 years ago
I also added a change to IsRTLLanguage that I have been meaning to make for some time and which is possible now that 9x isn't supported.
Attachment #228843 - Attachment is obsolete: true
Attachment #229071 - Flags: superreview?(roc)
Attachment #229071 - Flags: review?(roc)
Attachment #228843 - Flags: superreview?(roc)
Attachment #228843 - Flags: review?(roc)
Assignee

Comment 29

13 years ago
(In reply to comment #13)
> The other side of the coin is that BiDi users should never encounter a
> non-directional caret. Simon, should this be a new bug?

Better make it a new bug, since there doesn't seem to be consensus that it's the right thing to do. The fix would be very easy, basically just removing the marked lines in 
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/base/nsCaret.cpp&rev=1.160&mark=1091-1098#1078
and tidying up, but right now that would mean problems on Mac.
Attachment #229071 - Flags: superreview?(roc)
Attachment #229071 - Flags: superreview+
Attachment #229071 - Flags: review?(roc)
Attachment #229071 - Flags: review+
Assignee

Comment 30

13 years ago
Checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED

Comment 31

13 years ago
Verified FIXED using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060724 Minefield/3.0a1. 

I no longer see the bidi caret in any of the testcases or on the Wikipedia page itself.
Status: RESOLVED → VERIFIED
Assignee

Comment 32

13 years ago
*** Bug 346676 has been marked as a duplicate of this bug. ***

Comment 33

13 years ago
(In reply to comment #28)
> Created an attachment (id=229071) [edit]
> addressed review comments
> 
> I also added a change to IsRTLLanguage that I have been meaning to make for
> some time and which is possible now that 9x isn't supported.

This means the patch can't be used for branch, as branch still supports Win9x ? 

Assignee

Comment 34

13 years ago
Without the changes to nsBidiKeyboard::IsRTLLanguage(), it should be fine for branch.
*** Bug 352294 has been marked as a duplicate of this bug. ***

Comment 36

13 years ago
(In reply to comment #34)
> Without the changes to nsBidiKeyboard::IsRTLLanguage(), it should be fine for
> branch.

Could you please fix it in the branch?

Flags: blocking1.8.1?
Assignee

Comment 37

13 years ago
Posted patch Branch patchSplinter Review
This also includes a few lines from bug 272096, which are essential to make it work.

This is well-baked on trunk and low-risk, and many users complain about the Bidi caret indicator.
Attachment #237977 - Flags: approval1.8.1?

Comment 38

13 years ago
(In reply to comment #34)
> Without the changes to nsBidiKeyboard::IsRTLLanguage(), it should be fine for
> branch.
> 

Isn't this breaking win98 compatibility for branch in patch Attachment 237977 [details] [diff]?

 NS_IMETHODIMP nsBidiKeyboard::IsLangRTL(PRBool *aIsRTL)
 {
   *aIsRTL = PR_FALSE;
+
+  nsresult result = SetupBidiKeyboards();
+  if (NS_FAILED(result))
+    return result;
Assignee

Comment 39

13 years ago
(In reply to comment #38)

> Isn't this breaking win98 compatibility for branch in patch Attachment 237977 [details] [diff] [edit]?

Unless I am missing something, the only non-win98 compatible part of the original patch (attachment 229071 [details] [diff] [review]) was the call to ::GetLocaleInfoW(), which is not in this patch. It has been tested on German W2K with VC6.

Comment 40

13 years ago
(In reply to comment #39)
> (In reply to comment #38)
> 
> > Isn't this breaking win98 compatibility for branch in patch Attachment 237977 [details] [diff] [edit] [edit]?
> 
> Unless I am missing something, the only non-win98 compatible part of the
> original patch (attachment 229071 [details] [diff] [review] [edit]) was the call to ::GetLocaleInfoW(), which is
> not in this patch. It has been tested on German W2K with VC6.

Did you read my question in comment 33 and your answer in comment 34 regarding win98 compatibility of the patch attached in comment 28?

If you can give me a download address I can test on win98.

Comment 41

13 years ago
Comment on attachment 237977 [details] [diff] [review]
Branch patch

minusing the patch for drivers.   Concerned about risk and insufficient testing coverage this late in the release.
Attachment #237977 - Flags: approval1.8.1? → approval1.8.1-
Assignee

Comment 42

13 years ago
(In reply to comment #40)
> Did you read my question in comment 33 and your answer in comment 34 regarding
> win98 compatibility of the patch attached in comment 28?

I still don't see the connection. IsLangRTL() and IsRTLLanguage() are two different things.

Comment 43

13 years ago
(In reply to comment #42)
> (In reply to comment #40)
> > Did you read my question in comment 33 and your answer in comment 34 regarding
> > win98 compatibility of the patch attached in comment 28?
> 
> I still don't see the connection. IsLangRTL() and IsRTLLanguage() are two
> different things.

o.k., you're right, as I often have difficulties (in my own language) unconsciously picking the word 'right' or 'left', I didn't see the difference between IsLangRTL() and IsRTLLang...() comparing text in three tabs. Imho that's collateral damage from promgramming and debugging 80286 Assembler, often looking at hex numbers. You are used to swapping the nibbles automatically. Got to read slower, to not miss the end of the word, and threetimes.

Comment 44

13 years ago
 NS_IMETHODIMP nsBidiKeyboard::IsLangRTL(PRBool *aIsRTL)
 {
   *aIsRTL = PR_FALSE; <-- is it safe? or it should be checked against NULL first?
Flags: blocking1.8.1? → blocking1.8.1-
Assignee

Comment 45

13 years ago
(In reply to comment #44)
>  NS_IMETHODIMP nsBidiKeyboard::IsLangRTL(PRBool *aIsRTL)
>  {
>    *aIsRTL = PR_FALSE; <-- is it safe? or it should be checked against NULL
> first?

Please file a new bug
*** Bug 359884 has been marked as a duplicate of this bug. ***

Updated

11 years ago
Component: Layout: BiDi Hebrew & Arabic → Layout: Text
QA Contact: zach → layout.fonts-and-text
Assignee

Comment 48

11 years ago
(In reply to comment #13)
> The other side of the coin is that BiDi users should never encounter a
> non-directional caret. Simon, should this be a new bug?

This is now happening in bug 418513.
You need to log in before you can comment on or make changes to this bug.