753458 - Unnecessary fields added in bug 746633

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

The patch at https://bug746633.bugzilla.mozilla.org/attachment.cgi?id=621178 introduces a new screenSize and mSurfaceWidth/mSurfaceHeight fields, which I believe are redundant. The viewport width and height (as obtained by viewport.width() and viewport.height()) should be identical to these values, as they will always be the screen width/height in device pixels.

The extra fields added should be removed, or if in fact there is some reason they are needed, they should be commented as such.

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Remove unnecessary fields in RenderContext

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 626545 [details] [diff] [review]
Remove unnecessary fields in RenderContext

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

A very enthusiastic r+ from me :)

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2c51ac146815

Comment 4

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/2c51ac146815

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 626545 [details] [diff] [review]
Remove unnecessary fields in RenderContext

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: none
Testing completed (on m-c, etc.): on m-c for a while
Risk to taking this patch (and alternatives if risky): need this patch in order for the patches from bug 758635 to apply properly. (this patch also currently doesn't apply cleanly on beta but the rebase seems straightforward enough)
String or UUID changes made by this patch: none

Comment 6

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 626545 [details] [diff] [review]
Remove unnecessary fields in RenderContext

[Triage Comment]
As in bug 758635, please go ahead and land this to mozilla-beta tip only.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Landed on default branch of mozilla-beta:

https://hg.mozilla.org/releases/mozilla-beta/rev/6333f82c041e