1755647 - RDM doesn't fire change event of screen.orientation

Status: RESOLVED FIXED

(DevTools :: Responsive Design Mode, defect, P3)

Description

[Makoto Kato [:m_kato]]

What were you doing?

1. https://makotokato.github.io/demo/orientation.html
2. Use RDM in devtools
3. Click rotation icon

What happened?

orientationchange event on window object is fired, but change event on screen.orientation object isn't fired.

What should have happened?

change event on screen.orientation object isn't fired.

I guess that we should simulate change event in simulateScreenOrientationChange or _setRDMPaneOrientation

Comment 1

[Julian Descottes [:jdescottes]]

Thanks, we might have to fix this in https://searchfox.org/mozilla-central/rev/9bed2623831804ac086bbb80cb666e5c3b00416a/docshell/base/BrowsingContext.h#608

Comment 2

[Julian Descottes [:jdescottes]]

Nicolas, for investigation

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

So I guess we should call dom/base/ScreenOrientation.cpp#576-592 from docshell/base/BrowsingContext.h#608.

Micah, I see you modified BrowsingContext.h not too long ago , do you think you could take care of this issue ? If not, do you know we should ask for this (maybe calu) ? Thanks!

Comment 4

[Micah [:mtigley] (she/her)]

I can take a look into this. Thanks Nicolas!

Comment 5

[Emilio Cobos Álvarez (:emilio)]

nsGlobalWindowInner::Orientation seems like it won't look at the BrowsingContext orientation either.

Can we do the browsingcontext orientation thing as a sort-of override rather than keeping the values stored in the browsing context for all documents? That might be simpler.

Comment 6

[Micah [:mtigley] (she/her)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #5)

nsGlobalWindowInner::Orientation seems like it won't look at the BrowsingContext orientation either.

Can we do the browsingcontext orientation thing as a sort-of override rather than keeping the values stored in the browsing context for all documents? That might be simpler.

If I remember correctly, the orientation values were stored on the browsing context so that they persisted across refreshes while in RDM. I'm not entirely sure if there's now/already another way to do this.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Sure, and that's totally fine. My point is that right now we're treating the BrowsingContext values as the source of truth even when RDM is disabled. It'd be slightly easier to follow if this worked like other overrides, e.g.:

if (Maybe<uint16_t> angle = bc->GetOverrideAngle()) {
  return *angle;
}
return ScreenAngle();

But anyways I guess that's sort of an implementation detail that can be changed at any time :)

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 9

[Rubicon [:rubicon]]

Attachment: Bug 1755647 - Fire change event on window.screen.orientation. r=nchevobbe.

The change event is fired in ResponseActor.dispatchOrientationChangeEvent, along side the orientationchange event.

Comment 10

[Rubicon [:rubicon]]

Attachment: Bug 1755647 - Fire change event on window.screen.orientation. r=nchevobbe.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4188f5eb8240
Fire change event on window.screen.orientation. r=devtools-reviewers,nchevobbe.

Comment 12

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/4188f5eb8240