Word with "text-align: justify" moves as text selection changes

RESOLVED FIXED in Firefox 50



2 years ago
2 years ago


(Reporter: Matthew Kogan, Assigned: xidorn)



36 Branch

Firefox Tracking Flags

(firefox50 fixed)


MozReview Requests


Submitter Diff Changes Open Issues Last Updated
Error loading review requests:


(1 attachment)



2 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:47.0) Gecko/20100101 Firefox/47.0
Build ID: 20160604131506

Steps to reproduce:

- Open https://www.bmw-warranty.co.uk/Comprehensive#/_ComprehensiveDetails
- Start selecting text from the beginning of the second line of the paragraph underneath the bullet points, beginning "aware"
- Extend the selection slowly to the right

Actual results:

The word "number" later in that line moves slightly left and right as the selection changes.

Expected results:

No words should move.

Comment 1

2 years ago
I'm not able to reproduce it.

Does it work in safe mode?

With a fresh profile?
Component: Untriaged → Selection
Flags: needinfo?(matthew.bugzilla)
Product: Firefox → Core

Comment 2

2 years ago
I can reproduce it in safe mode and with a fresh profile.
Flags: needinfo?(matthew.bugzilla)

Comment 3

2 years ago
I could reproduce it in two different versions: Firefox Nightly 47 and Firefox Release version.


Comment 4

2 years ago
(In reply to Moin Shaikh from comment #3)
> I could reproduce it in two different versions: Firefox Nightly 47 and
> Firefox Release version.
> [testday-20160624]

Sorry, I mean Firefox 48.0b3 not the Release version.

Comment 5

2 years ago
Ok, I'm able to reproduce it if I set back the resolution to 100%.
The <p> has "text-align: justify".

Regression range:

Regressed by bug:
Xidorn Quan — Bug 1063857 - Improve selecting on justified characters. r=roc
Blocks: 1063857
Has Regression Range: --- → yes
Has STR: --- → yes
Ever confirmed: true
Flags: needinfo?(bugzilla)
Keywords: regression
Summary: Word moves as text selection changes → Word with "text-align: justify" moves as text selection changes
Version: 47 Branch → 36 Branch

Comment 6

2 years ago
Thanks for this great reproducible example. I eventually see where the issue is. Patch's coming.
Assignee: nobody → bugzilla
Flags: needinfo?(bugzilla)

Comment 7

2 years ago
Created attachment 8765335 [details]
Bug 1281457 - Compute justification spacing eagerly in SetupJustificationSpacing.

Before this change, SetupJustificationSpacing calls ComputeJustification
to compute how justification gaps are assigned to characters, and store
them in the PropertyProvider instance. When GetSpacing is called, those
information would be used to compute actual spacings from justification
before/after each character.

The bug is that, GetSpacing did not take gaps before the given range
into account when computing the spacing, which leads to unstable results
when range varies because of ignorance of accumulated error.

This patch changes it to eagerly computing the actual spacings inside
SetupJustificationSpacing, so that GetSpacing just queries the result
from mJustificationSpacings.

Review commit: https://reviewboard.mozilla.org/r/60770/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/60770/
Attachment #8765335 - Flags: review?(jfkthame)
Attachment #8765335 - Flags: review?(jfkthame) → review+
Comment on attachment 8765335 [details]
Bug 1281457 - Compute justification spacing eagerly in SetupJustificationSpacing.


I'm not currently seeing the bug here (on OS X; may depend on platform/font details?), but the patch seems fine; thanks for including the explanation in the commit message.

Comment 9

2 years ago
Since it depends on the natural width of the text piece, it should be font-dependent, and probably also platform-dependent. I can reproduce it on Windows, but not OS X, as well. And in most of cases, the effect is just too subtle to be noticed.

I think I did see this issue before, but didn't find a stably-reproducible example, and didn't realize this is a regression from my change...

Comment 10

2 years ago
Pushed by xquan@mozilla.com:
Compute justification spacing eagerly in SetupJustificationSpacing. r=jfkthame

Comment 11

2 years ago
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.