Closed Bug 1370053 Opened 3 years ago Closed 3 years ago

Bidi reordering regression in gmail compose message

Categories

(Core :: Layout: Text and Fonts, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: smontagu, Assigned: jfkthame)

References

Details

Attachments

(2 files)

I didn't manage to create a reduced testcase for this, but I can reliably reproduce it in gmail with the display language set to an RTL language.

Steps to reproduce: 

start to compose a new message in gmail
type a single LTR character in the "To" textbox ("אל" in Hebrew), e.g. J

expected results:
Contacts whose name begins with J appear in a listbox, correctly ordered, with J in boldface

actual results:
The bold J appears at the end of the contacts' names, e.g. "ohn SmithJ"

Regression range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e45890951ce77c3df05575bd54072b9f300d77b0&tochange=d700dc054751333e0735f975fce3d3adf153c62a
Yes, I can reliably reproduce this in gmail, though I haven't come up with a simplified testcase yet.

The problem seems to arise from the code in ResolveParagraph that takes a short-circuit in the case of an empty textframe. I have a patch that I think may fix the issue; I've pushed a try job (https://treeherder.mozilla.org/#/jobs?repo=try&revision=adfce6684ead6efea845ecac0d9821308e972fdf) and will post it for review if that looks OK.
OK, this fixes the bug seen in gmail, and doesn't regress anything else (afaict so far...)
Attachment #8874740 - Flags: review?(smontagu)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
While we're here, a couple of typos caught my eye. (No actual code changes here.)
Attachment #8874741 - Flags: review?(smontagu)
Attachment #8874740 - Flags: review?(smontagu) → review+
Attachment #8874741 - Flags: review?(smontagu) → review+
Could you add a test for it?
I'd like to, but so far I haven't come up with a reasonable-sized testcase (i.e. something less than interacting with the Compose panel on a Gmail page!) that reproduces the bug. Simply trying to recreate a similar frame subtree doesn't seem to be sufficient; I think it may be dependent on exactly how it gets dynamically created. (I guess that's not totally surprising, considering that bug 489887 -- where the special handling of an empty text node was originally added -- was about inconsistency after a dynamic change.)
https://hg.mozilla.org/integration/mozilla-inbound/rev/c474a1264de73553754f9d3570613cf2be594148
Bug 1370053 - Don't do the empty-frame short-circuit in ResolveParagraph until after updating the current bidi run if necessary. r=smontagu

https://hg.mozilla.org/integration/mozilla-inbound/rev/34eb6c45444316f0b9e0a86f0b7a22aa1d3ae86f
Bug 1370053 - Fix a couple of comment/variable-name typos in ResolveParagraph. r=smontagu
https://hg.mozilla.org/mozilla-central/rev/c474a1264de7
https://hg.mozilla.org/mozilla-central/rev/34eb6c454443
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Depends on: 1426042
You need to log in before you can comment on or make changes to this bug.