There is an #ifdef at http://mxr.mozilla.org/mozilla-central/source/layout/base/nsDisplayList.cpp?rev=a61d898ea4fa#703 which will need to be removed or updated once bug 732971 is landed. Because the resolution is set on a layer higher in the layer tree than the scrollable layer I'm not 100% sure the comment there is right, but it should be easy enough to check if that statement is a no-op and remove it if it is.
Bug 732971 has landed, so it seems like a good idea to sort this out now rather than later. kats, can you take this?
Yup I'll take this. For the record I did check to see if the statement in question was a no-op and I don't think it is. At least the values on the LHS and RHS of the assignment are not the same. I didn't check to see if it has effects later in the code but I assume it does. Removing the ifdef won't be as trivial as I thought.
Assignee: nobody → bugmail.mozilla
Merge the ifdef into the condition that determines the resolution. Now that bug 732971 is landed, this has no effect on B2G and Fennec. With my current patch queue for bug 902505, I was seeing the same problem on Metro that I was seeing previously on Fennec (i.e. all of the FrameMetrics had a mResolution == 1.0) and this patch fixes that too.
8 years ago
Comment on attachment 825926 [details] [diff] [review] Patch This is kind of nitty but would it be better to split this into fix + remove the hack in two patches? Will make it clearer in the history, and since we've had a lot of charges here recently, that would probably be appreciated.
Comment on attachment 825926 [details] [diff] [review] Patch Yeah I'll split it into two. Also as discussed on IRC I want to populate the FrameMetrics for all subdocument container layers, and so this will be affected by that as well (I'll want to set the mResolution only on the root frame of presShells rather than the rootScrollFrame).
8 years ago
Comment on attachment 827119 [details] [diff] [review] Part 1 - Set the resolution on the root scrollable layer for a presshell This makes sense. Can you explain what exactly goes wrong when looking for the root scroll id versus checking if we have the root scroll frame directly?
Attachment #827119 - Flags: review?(tnikkel) → review+
Attachment #827120 - Flags: review?(tnikkel) → review+
Wrote a small essay into the commit message of the first patch and landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/f34a58fcc1a3 https://hg.mozilla.org/integration/mozilla-inbound/rev/de45f494f6b2
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in before you can comment on or make changes to this bug.