remove unnecessary physical setters from logical-coordinate classes

RESOLVED FIXED in Firefox 42

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jfkthame, Assigned: jfkthame)

Tracking

unspecified
mozilla42
Points:
---

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(2 attachments)

Spun off from bug 1180643 comment 3. With only a little extra work, we can eliminate all use of physical setters for the various logical-coordinate classes, and then remove them from the WritingModes header altogether.
These conversions seem pretty simple, but I'll push a try job to check for unexpected breakage.
Attachment #8630416 - Flags: review?(smontagu)
Attachment #8630416 - Flags: review?(smontagu) → review+
Comment on attachment 8630417 [details] [diff] [review]
part 2 - Remove the unused physical setter methods on logical-coordinate classes

Review of attachment 8630417 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/WritingModes.h
@@ -673,5 @@
>      CHECK_WRITING_MODE(aWritingMode);
>      return mPoint.y;
>    }
>  
> -  nscoord X(WritingMode aWritingMode, nscoord aContainerWidth) const

I'm inclined to restrict the fix to removing the setters, and keep the const physical accessors even if they don't have any callers right now.

@@ -682,5 @@
> -    } else {
> -      return aWritingMode.IsBidiLTR() ? I() : aContainerWidth - I();
> -    }
> -  }
> -  nscoord Y(WritingMode aWritingMode) const

same here
Attachment #8630417 - Flags: review?(smontagu) → review+
(In reply to Simon Montagu :smontagu from comment #4)
> I'm inclined to restrict the fix to removing the setters, and keep the const
> physical accessors even if they don't have any callers right now.

Ah, right - that's what I meant to do, just got a little over-enthusiastic. :) Fixed before pushing, thanks.
https://hg.mozilla.org/mozilla-central/rev/e70e937ae75b
https://hg.mozilla.org/mozilla-central/rev/e3ef946cce23
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.