Closed
Bug 1034337
Opened 10 years ago
Closed 10 years ago
need to make bidi.edit.delete_immediately true by default to restore previous backspace behavior
Categories
(Core :: DOM: Editor, defect)
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: yuv.adm, Assigned: smontagu, NeedInfo)
References
Details
(Keywords: regression)
Attachments
(2 files)
5.80 KB,
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
1.14 KB,
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:30.0) Gecko/20100101 Firefox/30.0 (Beta/Release)
Build ID: 20140605174243
Steps to reproduce:
Go to any input field in the browser (URL bar, search bar, or any input field on a web page) and type in a mixed English/Hebrew text, or paste the following example:
abcdefgאבגדהוזחhijkl
Set the cursor on or around the Hebrew text and try to delete characters using backspace.
Actual results:
It takes two backspace strokes to delete a single character.
Expected results:
Naturally, a single stroke should delete a single character.
Comment 1•10 years ago
|
||
Regressed since Firefox7
Status: UNCONFIRMED → NEW
Component: Untriaged → Editor
Ever confirmed: true
Keywords: regression
OS: Mac OS X → All
Product: Firefox → Core
Version: 30 Branch → 7 Branch
Comment 2•10 years ago
|
||
Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/0c94f01f53af
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110704 Firefox/7.0a1 ID:20110704211816
Bad:
http://hg.mozilla.org/mozilla-central/rev/3f27dc203e62
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110705 Firefox/7.0a1 ID:20110705030811
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0c94f01f53af&tochange=3f27dc203e62
Regressed by:
3f27dc203e62 Simon Montagu — Undefine caret bidi level during reflow instead of on text entry. Bug 664087, r=roc
Blocks: 664087
status-firefox30:
--- → affected
status-firefox31:
--- → affected
status-firefox32:
--- → affected
status-firefox33:
--- → affected
status-firefox-esr24:
--- → affected
Assignee | ||
Comment 3•10 years ago
|
||
This is really expected behaviour -- if it didn't happen before bug 664087 that was a bug -- but you can change it by setting bidi.edit.delete_immediately to true in about:config
Assignee | ||
Comment 4•10 years ago
|
||
See bug 328834 for more info
Reporter | ||
Comment 5•10 years ago
|
||
:smontagu sorry I fail to see how that solves this bug. Out of the box users who type in Hebrew will need to hit backspace twice to delete. Why isn't delete_immediately the default behavior?
Flags: needinfo?(smontagu)
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Yuval Adam from comment #5)
> Why isn't
> delete_immediately the default behavior?
Please read bug 328834 for a full explanation. The spec linked to in that bug doesn't exist at the given URI, but there's a version of it at http://www.qsm.co.il/Hebrew/logicUI22.htm
That said, I see when experimenting with the testcase on different builds that what we are doing is not what the spec says: for example the caret doesn't move at all on the first backspace which undermines a large part of the rationale for the double backspace. ehsan, what is your opinion on the debate here and in bug 328834?
Flags: needinfo?(smontagu) → needinfo?(ehsan.akhgari)
Assignee | ||
Comment 8•10 years ago
|
||
I am leaning more and more to changing our behaviour here, especially since we already did so without noticing for some time, and changing back is perceived by users as a regression (here and in bug 1057552). Not to mention for interoperability with other browsers and mobile OSs.
There are a number of possibilities:
1) Remove bidi.edit.delete_immediately and the logic handling it completely.
2) Set bidi.edit.delete_immediately to true by default
3) Set bidi.edit.delete_immediately to true by default on some platforms (e.g. mobile) only
Any others?
Summary: Erroneous backspace handling for Hebrew text → Consider making bidi.edit.delete_immediately true by default (or just remove the pref)
Comment 9•10 years ago
|
||
I think we should switch bidi.edit.delete_immediately to true by default. That seems to be the least surprising UX here. Our current behavior is definitely broken.
Flags: needinfo?(ehsan.akhgari)
Assignee | ||
Comment 10•10 years ago
|
||
OK, let's do that. Try push at https://tbpl.mozilla.org/?tree=Try&rev=fb84895fb960.
I've filed bug 1067788 and bug 1067796 on other issues that I noticed with the caret in bidi text.
Assignee | ||
Comment 11•10 years ago
|
||
I'm actually pleased that this broke tests, because I suspected that there weren't any tests for this behaviour :)
The "Tests for bug 419406" in layout/generic/test/test_backspace_delete.xul need to set bidi.edit.delete_immediately back to false (or, better, test the behaviour with both values of the pref).
Comment 12•10 years ago
|
||
(In reply to Simon Montagu :smontagu from comment #11)
> I'm actually pleased that this broke tests, because I suspected that there
> weren't any tests for this behaviour :)
Haha, me too!
> The "Tests for bug 419406" in layout/generic/test/test_backspace_delete.xul
> need to set bidi.edit.delete_immediately back to false (or, better, test the
> behaviour with both values of the pref).
Let's do the latter.
Assignee | ||
Comment 13•10 years ago
|
||
This tests deletion with the pref set to both true and false, and also adds a new test so that we are testing Backspace as well as Delete.
Assignee: nobody → smontagu
Attachment #8491323 -
Flags: review?(ehsan.akhgari)
Assignee | ||
Comment 14•10 years ago
|
||
Attachment #8491325 -
Flags: review?(ehsan.akhgari)
Assignee | ||
Comment 15•10 years ago
|
||
Try push for those two patches: https://tbpl.mozilla.org/?tree=Try&rev=0396ebcb7051
Updated•10 years ago
|
Attachment #8491323 -
Flags: review?(ehsan.akhgari) → review+
Updated•10 years ago
|
Attachment #8491325 -
Flags: review?(ehsan.akhgari) → review+
Assignee | ||
Comment 16•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/434e148f58c1
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c737d160d5c
Flags: in-testsuite+
Comment 17•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/434e148f58c1
https://hg.mozilla.org/mozilla-central/rev/8c737d160d5c
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Comment 18•10 years ago
|
||
Not really sure about the reporting a regression bug in bugzilla, but this bug reproduced for me after installing Firefox 34.0.5 on Linux Mint 16 (today, Dec 13th 2014).
I used the following method to install the new Firefox version: http://www.libre-software.net/how-to-install-firefox-on-ubuntu-linux-mint
As stated previously by Yuval:
Steps to reproduce:
1. Enter Hebrew text in the search bar / URL bar. For example: פיזבאז
2. Press backspace
Actual Result:
For each two backspace keystrokes, one character is deleted.
Expected:
Each keystroke deletes one character,
I switched the bidi.edit.delete_immediately Boolean to True, which resolved the issue. This should be True by default - if it indeed does not affect the behavior of other components (everything seems to be working fine for me after the change, so far.
Reporter | ||
Comment 19•10 years ago
|
||
As far as I can tell neither of the patches have actually landed in Firefox 34. It's unclear to me if they should have or not.
Comment 23•10 years ago
|
||
Hi,
Sorry to barge in on this, but
Comment 24•10 years ago
|
||
[sorry about that goof above, couldn't find the delete function]
Hi,
Sorry to barge in on this, but I fail to see the *exact* connection between bidi.edit.delete_immediately and the problem that Yuval Adam has reported.
The way I understand it bidi.edit.delete_immediately is supposed to offer alternate behaviors of backspace when deleting characters _between_ _direction_ _boundraries_. In other words: it's only supposed to affect a backspace key pressed when the text in front of the cursor was written in one direction (say, left-to-right) and the text in rear of the cursor was written in the other direction (say, right-to-left).
In this case the description is of something *else*: when deleting hebrew text, even if there is _no_ change in direction, _every_ character deletion requires _two_ backspace clicks.
Or am I missing something here?
Reporter | ||
Comment 25•10 years ago
|
||
(In reply to Amir Meiri from comment #24)
> In this case the description is of something *else*: when deleting hebrew
> text, even if there is _no_ change in direction, _every_ character deletion
> requires _two_ backspace clicks.
Well, clear the URL bar and type a Hebrew string.
If you try to delete from the end of that string indeed you get the double backspace problem.
However, if you move the cursor to the middle of the string, you get proper single backspace behavior.
So, there seems to be an implicit direction change at the end of a bidi string.
In any case, this bug still exists.
Flags: needinfo?(smontagu)
Assignee | ||
Comment 26•10 years ago
|
||
(In reply to Amir Meiri from comment #24)
> In this case the description is of something *else*: when deleting hebrew
> text, even if there is _no_ change in direction, _every_ character deletion
> requires _two_ backspace clicks.
> Or am I missing something here?
You're 100% right, that is what Ehsan and I meant above when we agreed that the current behaviour was broken. The bug still exists only if you see it in a nightly build, or with bidi.edit.delete_immediately set to true in about:config
Flags: needinfo?(smontagu)
Comment 27•10 years ago
|
||
OK, I see what you mean Yuval. I didn't consider the end-of-line condition to be a direction switch, but I guess that's potato/potahto.
Comment 30•10 years ago
|
||
Hello dears
This bug status is resolved
I use latest version of firefox (34.0.5)
It is not resolved yet.
Comment 31•10 years ago
|
||
I have bidi.edit.delete_immediately set to false and it still happens in the address bar and search inputs, seems resolved in web pages themselves.
FF 34.0.5 latest update.
tried to change it to true it didn't make a difference.
I still need two backspaces to delete one hebrew character in address bar and search input.
Flags: needinfo?(smontagu)
See Also: → https://launchpad.net/bugs/1405930
Updated•10 years ago
|
Summary: Consider making bidi.edit.delete_immediately true by default (or just remove the pref) → need to make bidi.edit.delete_immediately true by default to restore previous backspace behavior
Comment 34•10 years ago
|
||
I changed the flag manually and it seems to have solved the problem but after this I am experiencing a bug that I am not sure if it is related to changing this flag or not.
When I have multiple tabs open, sometimes the tabs wont open when I click on them, meaning that Firefox shows that a new tab is clicked by bringing it forward but the page under the tab remains from the previous opened tab.
Does anybody know what is causing this?
p.s. Hardware acceleration is turned off in my Firefox settings. Everything else is set to default.
You need to log in
before you can comment on or make changes to this bug.
Description
•