Closed
Bug 288839
Opened 20 years ago
Closed 18 years ago
ltr caret in input field unexpected (directional caret appears when only ltr input methods are installed)
Categories
(Core :: Layout: Text and Fonts, defect)
Core
Layout: Text and Fonts
Tracking
()
VERIFIED
FIXED
People
(Reporter: santiago.matamoros, Assigned: smontagu)
References
(Blocks 1 open bug, )
Details
(Keywords: testcase)
Attachments
(5 files, 1 obsolete file)
(deleted),
text/html
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mtschrep
:
approval1.8.1-
|
Details | Diff | Splinter Review |
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).
Comment 1•20 years ago
|
||
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.
Comment 2•20 years ago
|
||
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•20 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•20 years ago
|
||
Moving to Core->Layout: Form Controls
Component: General → Layout: Form Controls
Product: Firefox → Core
Version: unspecified → Trunk
Comment 5•20 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•20 years ago
|
Assignee: firefox → mkaply
QA Contact: general → zach
Comment 6•20 years ago
|
||
Comment 7•20 years ago
|
||
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•20 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•20 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/">العربية</a> </div>
Assignee | ||
Comment 10•20 years ago
|
||
Mano, did we discuss somewhere never showing the directional caret if there is no RTL keyboard installed?
Comment 11•20 years ago
|
||
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•20 years ago
|
||
Mano/smontagu: So, like, are you confirming this bug, marking it as a dupe or what?
Comment 13•20 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•20 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•20 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.
Comment 20•18 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•18 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 23•18 years ago
|
||
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•18 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.
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•18 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•18 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•18 years ago
|
||
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Comment 31•18 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
Comment 33•18 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•18 years ago
|
||
Without the changes to nsBidiKeyboard::IsRTLLanguage(), it should be fine for branch.
Comment 36•18 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•18 years ago
|
||
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•18 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•18 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•18 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•18 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•18 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•18 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•18 years ago
|
||
NS_IMETHODIMP nsBidiKeyboard::IsLangRTL(PRBool *aIsRTL) { *aIsRTL = PR_FALSE; <-- is it safe? or it should be checked against NULL first?
Updated•18 years ago
|
Flags: blocking1.8.1? → blocking1.8.1-
Assignee | ||
Comment 45•18 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. ***
Component: Layout: BiDi Hebrew & Arabic → Layout: Text
QA Contact: zach → layout.fonts-and-text
Assignee | ||
Comment 48•16 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.
Description
•