The default bug view has changed. See this FAQ.

Bidi: Caret navigation is broken inside inline elements

RESOLVED FIXED in mozilla1.8rc1

Status

()

Core
Layout: Text
RESOLVED FIXED
12 years ago
9 years ago

People

(Reporter: Uri Bernstein (Google), Assigned: Uri Bernstein (Google))

Tracking

({fixed1.8, regression})

Trunk
mozilla1.8rc1
fixed1.8, regression
Points:
---
Bug Flags:
blocking1.8rc1 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

12 years ago
When I fixed bug 288789 by changing the semantics of
nsFrameList::GetPrev[Next]VisualFor(), I neglected to change the case where the
parent frame is not a block frame (i.e. it's an inline frame). This has no
effect on textareas (which can't contain inlines), but it does break HTML
editing, keyboard-selection, and caret navigation when bidirectional text is
contained within an inline element.

I'll attach a testcase and a patch soon.
(Assignee)

Comment 1

12 years ago
Created attachment 200231 [details]
testcase

In Composer, or Caret Browsing mode:

In the upper paragraph, try moving the caret in and out of the numbers (in the
1st and 3rd lines). Notice that the caret moves to all kinds of unexpected
places.

The lower paragraph (which is idetical except it is not wrapping a <span>)
works correctly.
Assignee: mozilla → uriber
Status: NEW → ASSIGNED
(Assignee)

Comment 2

12 years ago
Created attachment 200233 [details] [diff] [review]
patch

In the case where the parent is not a block frame, use the direction-aware
nsFrameOrigin (with a dummy value of "0" for the line number), instead of just
comparing the x coordinates (disregarding direction).

Simon - do you think you can review this soon so we will have some sort of
chance of getting this into 1.8? (I'm not too optimistic, but let's try).
Attachment #200233 - Flags: review?(smontagu)
Comment on attachment 200233 [details] [diff] [review]
patch

r=smontagu
Attachment #200233 - Flags: review?(smontagu) → review+
(Assignee)

Comment 4

12 years ago
Comment on attachment 200233 [details] [diff] [review]
patch

Thanks, Simon!

ROC - same request here. This is a regression, and I'd really like to try and
slip the fix into 1.5.
Attachment #200233 - Flags: superreview?(roc)
Comment on attachment 200233 [details] [diff] [review]
patch

looks good ... it's pretty clear that nothing has changed for LTR too
Attachment #200233 - Flags: superreview?(roc) → superreview+
(Assignee)

Comment 6

12 years ago
Comment on attachment 200233 [details] [diff] [review]
patch

Asking for last-minute approval1.8rc1. The changed code is only reached when
navigating in bidi HTML, so any risk is limited to that area (which is pretty
broken without this fix).
Attachment #200233 - Flags: approval1.8rc1?
(Assignee)

Updated

12 years ago
Flags: blocking1.8rc1?

Updated

12 years ago
Attachment #200233 - Flags: approval1.8rc1? → approval1.8rc1+

Comment 7

12 years ago
not going to flag this as a blocker but if it lands before the freeze, then it's
in :-)
Flags: blocking1.8rc1? → blocking1.8rc1-
(Assignee)

Comment 8

12 years ago
Checked in by timeless to both trunk and branch, 2005-10-21, 13:06.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
(Assignee)

Updated

12 years ago
Target Milestone: --- → mozilla1.8rc1

Updated

9 years ago
Component: Layout: BiDi Hebrew & Arabic → Layout: Text
QA Contact: zach → layout.fonts-and-text
You need to log in before you can comment on or make changes to this bug.