The default bug view has changed. See this FAQ.

Store the page size in FrameMetrics in CSS pixels in addition to device pixels.

RESOLVED FIXED in mozilla14

Status

()

Core
Layout
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jrmuizel, Assigned: jrmuizel)

Tracking

(Blocks: 1 bug)

unspecified
mozilla14
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-fennec1.0 beta+)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Some of the consumers (The Java frontend and RenderFrameParent) need the value in CSS pixels and using the device pixels gives slightly incorrect results because of rounding.
(Assignee)

Updated

5 years ago
Blocks: 742134
(Assignee)

Comment 1

5 years ago
Created attachment 614514 [details] [diff] [review]
Store the page size in FrameMetrics in CSS pixels in addition to device pixels
(Assignee)

Updated

5 years ago
Attachment #614514 - Flags: review?(jones.chris.g)
Comment on attachment 614514 [details] [diff] [review]
Store the page size in FrameMetrics in CSS pixels in addition to device pixels

What consumer wants CSS-pixel values, and how do they fix rounding errors?  This change on its own is fine, but I'm skeptical that it would legitimately fix anything.
Attachment #614514 - Flags: review?(jones.chris.g)
(Assignee)

Comment 3

5 years ago
(In reply to Chris Jones [:cjones] [:warhammer] from comment #2)
> Comment on attachment 614514 [details] [diff] [review]
> Store the page size in FrameMetrics in CSS pixels in addition to device
> pixels
> 
> What consumer wants CSS-pixel values, and how do they fix rounding errors? 
> This change on its own is fine, but I'm skeptical that it would legitimately
> fix anything.

The consumer that this patch fixes is RenderFrameParent. It seems to currently assume that rounding doesn't happen.

The Java front end currently uses device pixel page sizes to calculate new device pixel page size which is obviously wrong. This patch is a step toward fixing that.
(Assignee)

Updated

5 years ago
Attachment #614514 - Flags: review?(jones.chris.g)
Comment on attachment 614514 [details] [diff] [review]
Store the page size in FrameMetrics in CSS pixels in addition to device pixels

>diff --git a/gfx/layers/Layers.h b/gfx/layers/Layers.h

>+  // Consumer's often want to know the size before scaling to pixels
>+  // so we record this size as well.
>+  gfx::Size mCSSContentSize;
>+

I think this member is pretty self-explanatory, not sure this comment
is helpful.  (And it has a typo.)  Would drop it, up to you.
Attachment #614514 - Flags: review?(jones.chris.g) → review+

Updated

5 years ago
blocking-fennec1.0: --- → ?

Updated

5 years ago
Assignee: nobody → jmuizelaar

Comment 5

5 years ago
Please do a complete risk assessment of all three pieces before landing, it seems like we could break a lot here.
blocking-fennec1.0: ? → beta+
(Assignee)

Comment 6

5 years ago
This patch has very minimal risk, it adds some fields and uses them in one consumer in a place that was previously incorrect.
Blocks: 744916
(Assignee)

Comment 7

5 years ago
Created attachment 615998 [details] [diff] [review]
typo's fixed
Attachment #614514 - Attachment is obsolete: true
Attachment #615998 - Flags: review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/e9053acaa5e1 with the typos actually fixe'd.
https://hg.mozilla.org/mozilla-central/rev/e9053acaa5e1
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
Depends on: 749953
Depends on: 753762
You need to log in before you can comment on or make changes to this bug.