Closed
Bug 737274
Opened 12 years ago
Closed 11 years ago
Tab.setResolution should set the resolution of the contentWindow
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(blocking-fennec1.0 -)
RESOLVED
FIXED
Firefox 28
Tracking | Status | |
---|---|---|
blocking-fennec1.0 | --- | - |
People
(Reporter: blassey, Assigned: kats)
References
Details
Attachments
(1 file)
962 bytes,
patch
|
mfinkle
:
review+
kats
:
review-
|
Details | Diff | Splinter Review |
No description provided.
Attachment #607386 -
Flags: review?(mark.finkle)
Attachment #607386 -
Flags: review?(bugmail.mozilla)
Comment 1•12 years ago
|
||
Comment on attachment 607386 [details] [diff] [review] patch >- if (BrowserApp.selectedTab == this) { >- let cwu = window.top.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils); >- cwu.setResolution(aZoom, aZoom); >- } >+ let win = this.browser.contentWindow; >+ let cwu = win.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils); >+ cwu.setResolution(aZoom, aZoom); Using the content window, not the XUL window, does seem to be the "right" thing to do and matches what we did in XUL fennec. You dropped the "if this is the selected tab" check though. Do we want to set this regardless? It shouldn't hurt and we also do that in XUL fennec.
Attachment #607386 -
Flags: review?(mark.finkle) → review+
Reporter | ||
Comment 2•12 years ago
|
||
this is where the code went from setting resolution on the content document to setting it on the top level window: https://hg.mozilla.org/mozilla-central/rev/726cd11889e6
Assignee | ||
Comment 3•12 years ago
|
||
Comment on attachment 607386 [details] [diff] [review] patch Review of attachment 607386 [details] [diff] [review]: ----------------------------------------------------------------- I wish it were this easy. It doesn't work though, same as last time I tried it. If you build with this patch and pinch zoom, you'll see things scale up but that's only the compositor scaling what Gecko has drawn at a 1.0 resolution. Gecko doesn't actually draw the content at the resolution we give it.
Attachment #607386 -
Flags: review?(bugmail.mozilla) → review-
Assignee | ||
Comment 4•12 years ago
|
||
(In reply to Mark Finkle (:mfinkle) from comment #1) > Using the content window, not the XUL window, does seem to be the "right" > thing to do and matches what we did in XUL fennec. It is the "right" thing to do, but as far as I understand, it's not what XUL fennec did. XUL fennec did this: let rootCwu = content.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils); if (json.id == 1) { rootCwu.setResolution(json.scale, json.scale); where "content" is AIUI the top-level XUL window. (In reply to Brad Lassey [:blassey] from comment #2) > this is where the code went from setting resolution on the content document > to setting it on the top level window: > https://hg.mozilla.org/mozilla-central/rev/726cd11889e6 That change just moves around the line of code in question; both before and after that cset we're setting the resolution on the top-level XUL window.
Comment 5•12 years ago
|
||
(In reply to Kartikaya Gupta (:kats) from comment #4) > (In reply to Mark Finkle (:mfinkle) from comment #1) > > Using the content window, not the XUL window, does seem to be the "right" > > thing to do and matches what we did in XUL fennec. > > It is the "right" thing to do, but as far as I understand, it's not what XUL > fennec did. XUL fennec did this: > > let rootCwu = > content.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci. > nsIDOMWindowUtils); > if (json.id == 1) { > rootCwu.setResolution(json.scale, json.scale); > > where "content" is AIUI the top-level XUL window. Nope. In XUL Fennec (which was multi-process), "content" referred to the contentWindow of the Tab.
Assignee | ||
Updated•12 years ago
|
Updated•12 years ago
|
blocking-fennec1.0: --- → ?
blocks two beta bugs : 732089 732971
Reporter | ||
Updated•12 years ago
|
Assignee: nobody → blassey.bugs
Reporter | ||
Comment 7•12 years ago
|
||
tn thinks we can fix bug 732016 without this
No longer blocks: 732016
Comment 8•12 years ago
|
||
Re-noming to minus - 732016 can be fixed without this.
Blocks: 732016
blocking-fennec1.0: beta+ → ?
Reporter | ||
Updated•12 years ago
|
blocking-fennec1.0: ? → -
Updated•11 years ago
|
Assignee: blassey.bugs → bugmail.mozilla
Assignee | ||
Comment 9•11 years ago
|
||
This was fixed as part of bug 732971.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Target Milestone: --- → Firefox 28
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•