Mark Côté [:mcote] Reporter
	Description • 10 years ago

I have two Nightly windows open, one in my built-in Retina display, the other on an external (standard DPI) monitor.

When I drag a tab from the window on my built-in display to the window (or as a new window) on my external monitor, after switching away and back again due to bug 1095729, when the content displays it is zoomed in (huge), as if it didn't get readjusted for the resolution/DPI on my external monitor.  Similarly, dragging from the external monitor to the built in causes it to be zoomed out (tiny).

Dragging to yet another window on the destination display does not reset it.  Dragging it back to the original display resets the zoom (looks normal again).

	:Gijs (he/him)
	Comment 1 • 10 years ago

Does this reproduce outside of e10s mode?

	Mark Côté [:mcote] Reporter
	Comment 2 • 10 years ago

(In reply to :Gijs Kruitbosch from comment #1)
> Does this reproduce outside of e10s mode?

It does not.

	Mark Côté [:mcote] Reporter
	Comment 3 • 9 years ago

FWIW I found a workaround: drag the tab to a new window and then drag the window across from the first to the second monitor. At this point the tab can be dragged to an existing window on the second monitor.

	Neil Deakin
	Comment 4 • 9 years ago

So I think we need to call BackingScaleFactorChanged() when swapping docshells (in nsFrameLoader::SwapWithOtherRemoteLoader), as the non-remote case does, to update the screen resolution. However, the child doesn't receive the messages until after the size has been recomputed, resulting in an incorrect size.

So essentially, we need to wait for the remote child to finish processing the screen resolution change and then recompute the size.

	Neil Deakin
	Comment 5 • 9 years ago

The size change (UpdateDimensions) happens before the UI resolution call to the child. Here, I call UpdateDimensions again afterwards. I feel like there should be a better way to do this, avoiding setting the size twice.

Thoughts?

	Bill McCloskey [inactive unless it's an emergency] (:billm)
	Comment 6 • 9 years ago

Comment on attachment 8599992 [details] [diff] [review]
zoomtwoscreens

Sorry, I don't know anything about this. Maybe roc does.

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 7 • 9 years ago

Comment on attachment 8599992 [details] [diff] [review]
zoomtwoscreens

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

This seems related to David's bug. David, does your patch fix this?

	David Parks [:handyman]
	Comment 8 • 9 years ago

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #7)
> 
> This seems related to David's bug. David, does your patch fix this?

Nope.  But this patch does have the ingredient that makes it work... nsFrameLoader::SwapWithOtherRemoteLoader needs to refresh the backing scale.  The rest of the patch is unnecessary with the patch in bug 1125325.

Since this looks clear to me, presumptuous tests:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=25cacfb7057c

	Neil Deakin
	Comment 9 • 9 years ago

That try log shows failures. David, does your patch fix those issues?

Is it sufficient to use your patch in bug 1125325 and the BackingScaleFactorChanged patch here?

	David Parks [:handyman]
	Comment 10 • 9 years ago

The test failure includes the patch from 1125325 -- that patch seems to be the cause.  I'm looking into that.

	David Parks [:handyman]
	Comment 11 • 9 years ago

Update: I had that wrong -- this patch sparks the failure.  I'm unable to reproduce in a local Ubuntu debug build (it only fails on try in debug) but I have some ideas on whats happening.

	David Parks [:handyman]
	Comment 12 • 9 years ago

Tests (on top of patches from bug 1125325 -- this patch hasn't changed):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a00a1d6bd876

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 13 • 9 years ago

Hey Enn - mind if I try to drive this one home?

	Neil Deakin
	Comment 14 • 9 years ago

You be best to ask David about it, since most of the work to get this going is in bug 1125325. The only change here I think is the one in the patch is the change to nsFrameLoader.cpp

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 15 • 9 years ago

Right - so, as Enn pointed out (and for any future triage that cares to know), the patch in here is waiting for bug 1125325 to land.

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 16 • 9 years ago

Bug 1125325 has landed and bounced a few times. It's currently on central, and I'm going to give it a few more days before I land the patch in here.

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 17 • 9 years ago

Bug 1125325 has survived one day on central. I'm going to land this tomorrow if bug 1125325 doesn't get backed out.

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 18 • 9 years ago

Bug 1096550 - Update content scale when swapping remote tabs between windows. r=roc.

We need to do this in case the windows we are swapping between have different
content scales (ie. HiDPI vs LowDPI).

	Pulsebot
	Comment 19 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/ae9dce3bd47e

	Carsten Book [:Tomcat]
	Comment 20 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/ae9dce3bd47e

	Mark Côté [:mcote] Reporter
	Comment 21 • 9 years ago

I just upgraded Nightly to 2015-06-18 and dragged a tab from my built-in high-res monitor to my external standard-res monitor.  Still seeing the same behaviour; see attached screenshot.

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 22 • 9 years ago

mcote - were you dragging the tab _into_ another window? Or to it's own window?

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 23 • 9 years ago

To be specific - there are two cases:

1) Tearing a tab from hi-res to low-res in one gesture, such that a new window with the tab you're tearing out is created in the low-res window
2) Tearing a tab from hi-res and dragging it into a pre-existing browser window on the low-res screen

Which one matches your STR? Or neither?

	Mark Côté [:mcote] Reporter
	Comment 24 • 9 years ago

It was 2), but I just tried 1) as well, and it also results in magnification.

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 25 • 9 years ago

So I grabbed a Yosemite machine and could not reproduce this on the latest Nightly (2015-06-19).

Florin - I see that you tested the fix for bug 1125325. This one is very similar (see the sorta STR in comment 0 and comment 23).

Are you able at all to reproduce the behaviour that mcote is seeing?

	Camelia Badau [:cbadau], Desktop QA
	Comment 26 • 9 years ago

I've tested the two scenarios from comment 23 on a MacBook Pro Retina, 15-inch, Late 2013, with an external monitor and I have the following results: 
- on Nightly 2015-06-18, the behaviour from comment 23 is reproducible
- on latest Nightly 41.0a1 (2015-06-23), I wasn't able to reproduce it

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 27 • 9 years ago

Hey mcote - are you still seeing this with e10s enabled on Nightly?

If so, can you please record yourself doing the STR with a phone or something and post the video here so we can examine it? We're having no luck reproducing, so we feel like we're missing a piece of the puzzle.

	Mark Côté [:mcote] Reporter
	Comment 28 • 9 years ago

No, it's all good now.  Thanks! :D

	Petruta Horea [:phorea], Desktop QA
	Comment 29 • 9 years ago

Marking as verified based on comments 26 & 28.