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

VERIFIED FIXED

Status

()

VERIFIED FIXED
14 years ago
10 years ago

People

(Reporter: santiago.matamoros, Assigned: smontagu)

Tracking

(Blocks: 1 bug, {testcase})

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

Firefox Tracking Flags

(Not tracked)

Details

(URL)

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: 265070
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 6

14 years ago
Created attachment 179491 [details]
reduced testcase, original code

Comment 7

14 years ago
Created attachment 179492 [details]
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
Created attachment 179496 [details]
minimal testcase arab language

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
Created attachment 228843 [details] [diff] [review]
Windows patch
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
Created attachment 229071 [details] [diff] [review]
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.
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
Created attachment 237977 [details] [diff] [review]
Branch patch

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?
(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

10 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.