1819203 - Identical if and else clauses in nsCaret::GetGeometryForFrame

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Randell Jesup [:jesup] (needinfo me)]

coverity:

---

*** CID 1521542: Incorrect expression (IDENTICAL_BRANCHES)
/layout/base/nsCaret.cpp: 289 in nsCaret::GetGeometryForFrame(nsIFrame *, int, int *)()
283 // already, which reverses the desired caret placement behavior.
284 // Note that the following condition is equivalent to:
285 // if ( (!textRun->IsSidewaysLeft() && textRunDirIsReverseOfFrame) ||
286 // (textRun->IsSidewaysLeft() && !textRunDirIsReverseOfFrame) )
287 if (textRunDirIsReverseOfFrame != textRun->IsSidewaysLeft()) {
288 int dir = wm.IsBidiLTR() ? -1 : 1;

CID 1521542:  Incorrect expression  (IDENTICAL_BRANCHES)
The same code is executed regardless of whether "vertical" is true, because the 'then' and 'else' branches are identical. Should one of the branches be modified, or the entire 'if' statement replaced?

289 if (vertical) {
290 inlineOffset = dir * caretMetrics.mCaretWidth;
291 } else {
292 inlineOffset = dir * caretMetrics.mCaretWidth;
293 }
294 }

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Seems like the code is correct because vertical is accounted below, so we probably just want to remove the condition, but ni?ing David to confirm.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1815729

Comment 3

[David Shin[:dshin]]

Ah - yep, got rid of variable shuffle in D169210 by working in logical axis but missed that part.

Comment 4

[David Shin[:dshin]]

Attachment: Bug 1819203 - Remove identical if block introduced in D169210. r=#layout-reviewers

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b4175f1f21b4
Remove identical if block introduced in D169210. r=emilio

Comment 6

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/b4175f1f21b4