Closed
Bug 1177505
Opened 9 years ago
Closed 9 years ago
Couple of extra pixels on the caret when composing a tweet
Categories
(Core :: Layout: Text and Fonts, defect)
Tracking
()
People
(Reporter: alex_mayorga, Assigned: tedders1)
References
Details
(Whiteboard: [systemsfe])
Attachments
(2 files, 3 obsolete files)
271 bytes,
text/html
|
Details | |
2.41 KB,
patch
|
ritu
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
Steps: 1 When replying to a tweet on twitter.com 2 move the caret around the composed message Result: There are a couple odd pixels on the top right of the caret, see http://screencast.com/t/vajjlAAtvQDR Expected result: There are no extra pixels on the caret
Updated•9 years ago
|
Flags: needinfo?(tclancy)
Comment hidden (spam) |
Comment 2•9 years ago
|
||
Attachment #8626319 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → tclancy
Flags: needinfo?(tclancy)
Assignee | ||
Comment 3•9 years ago
|
||
Attachment #8626731 -
Flags: review?(smontagu)
Assignee | ||
Comment 4•9 years ago
|
||
Green treeherder: https://treeherder.mozilla.org/#/jobs?repo=try&revision=51a360bb4b21
Comment 5•9 years ago
|
||
Comment on attachment 8626731 [details] [diff] [review] bug-1177505-fix.patch Review of attachment 8626731 [details] [diff] [review]: ----------------------------------------------------------------- I'm concerned that this might cause regressions when deleting RTL text from an LTR paragraph. I want to test it for a while before r+ing.
Comment 6•9 years ago
|
||
Comment on attachment 8626731 [details] [diff] [review] bug-1177505-fix.patch Review of attachment 8626731 [details] [diff] [review]: ----------------------------------------------------------------- In a LTR textarea, type "abc<space>אבג<space>123" Expected and actual results visually from left to right: the three Latin characters followed by the three digits followed by the three Hebrew characters. Select the Hebrew characters and type "def" Expected results visually from left to right: abc def 123 Actual results visually from left to right: abc 123 fed This happens because when the Hebrew characters are present a bidi continuation to the text frame is created and given RTL directionality. Then after the Hebrew characters are no longer there the text is all LTR so runCount = 1. Before this change, because frameCount was > 1, we didn't make the early return from ResolveParagraph so the continuation frame was converted to a fluid continuation and restored to LTR directionality. With this change we do make the early return without checking frameCount, so the continuation frame still acts as RTL even though it only contains LTR characters.
Attachment #8626731 -
Flags: review?(smontagu) → review-
Assignee | ||
Updated•9 years ago
|
Iteration: --- → 42.1 - Jul 13
Updated•9 years ago
|
Whiteboard: [systemsfe]
Target Milestone: --- → FxOS-S2 (10Jul)
Assignee | ||
Comment 8•9 years ago
|
||
Attachment #8626731 -
Attachment is obsolete: true
Attachment #8639097 -
Flags: review?(smontagu)
Assignee | ||
Comment 9•9 years ago
|
||
Green treeherder: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3d3484095208
Comment 10•9 years ago
|
||
Comment on attachment 8639097 [details] [diff] [review] bug-1177505-fix.patch Review of attachment 8639097 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/base/nsBidiPresUtils.cpp @@ +741,4 @@ > aBpd->mParagraphDepth == 0 && aBpd->GetDirection() == NSBIDI_LTR && > aBpd->GetParaLevel() == 0 && > frame0 != NS_BIDI_CONTROL_FRAME && > !frame0->Properties().Get(nsIFrame::EmbeddingLevelProperty()) && You should probably add a null check for frame0 here, since you've removed the check on frameCount.
Attachment #8639097 -
Flags: review?(smontagu) → review+
Assignee | ||
Comment 11•9 years ago
|
||
Attachment #8639097 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 12•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/efd26ea929a0
Keywords: checkin-needed
Comment 14•9 years ago
|
||
Ted, should we uplift this fix into beta so that we can fix it for 41?
Flags: needinfo?(tclancy)
Assignee | ||
Comment 15•9 years ago
|
||
Yes, because Bug 1164693 is in Firefox 41, this should be in Firefox 41 too. I'll apply for uplift to beta.
Flags: needinfo?(tclancy)
Assignee | ||
Comment 16•9 years ago
|
||
Comment on attachment 8641282 [details] [diff] [review] bug-1177505-fix.patch Approval Request Comment [Feature/regressing bug #]: Bug 1164693 [User impact if declined]: The directional caret sometimes appears (haphazardly) on non-bidi webpages, like Twitter. Non-bidi users are sometimes confused by the appearance of the directional caret and mistake it for a visual rendering bug. [Describe test coverage new/current, TreeHerder]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3d3484095208 [Risks and why]: None foreseen. [String/UUID change made/needed]: None
Attachment #8641282 -
Flags: approval-mozilla-beta?
Alex, could you please verify the fix on a latest Nightly whenever you get a chance? Thanks.
Flags: needinfo?(alex_mayorga)
Comment on attachment 8641282 [details] [diff] [review] bug-1177505-fix.patch This patch has been in Nightly for almost a month now. Let's uplift to Beta41.
Attachment #8641282 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•9 years ago
|
QA Whiteboard: [good first verify]
Reporter | ||
Comment 20•9 years ago
|
||
(In reply to Ritu Kothari (:ritu) from comment #17) > Alex, could you please verify the fix on a latest Nightly whenever you get a > chance? Thanks. ¡Hola Ritu! WFM on Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:43.0) Gecko/20100101 Firefox/43.0 ID:20150828030205 CSet: 87e23922be375985d0b1906ed5ba5f095f323a38 See https://twitter.com/alex_mayorga/status/637317321984356352 ¡Gracias!
Status: RESOLVED → VERIFIED
Flags: needinfo?(alex_mayorga)
(In reply to alex_mayorga from comment #20) > (In reply to Ritu Kothari (:ritu) from comment #17) > > Alex, could you please verify the fix on a latest Nightly whenever you get a > > chance? Thanks. > > ¡Hola Ritu! > > WFM on Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:43.0) Gecko/20100101 > Firefox/43.0 ID:20150828030205 CSet: 87e23922be375985d0b1906ed5ba5f095f323a38 > > See https://twitter.com/alex_mayorga/status/637317321984356352 > > ¡Gracias! Thank you! :)
You need to log in
before you can comment on or make changes to this bug.
Description
•