Closed Bug 1296498 Opened 5 years ago Closed 5 years ago

Enable RDM redesign in Nightly only

Categories

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

defect

Tracking

(firefox52 fixed)

RESOLVED FIXED
Firefox 52
Iteration:
52.1 - Oct 3
Tracking Status
firefox52 --- fixed

People

(Reporter: jryans, Assigned: jryans)

References

Details

(Whiteboard: [multiviewport] [reserve-rdm])

Attachments

(2 files)

Tracking bug to collect things to be resolved before enabling new RDM in Nightly only (to gather more feedback before shipping to other channels).

Since it requires e10s, we may only enable when e10s is on.

The basic criteria here is to have general usability blockers resolved, such as the toolbox working as expected.  New features that were not present in old RDM aren't required to be done.
Flags: qe-verify-
Whiteboard: [multiviewport] [userstory]
Assignee: nobody → jryans
Status: NEW → ASSIGNED
I think we're now far enough along to flip on new RDM for Nightly only here.
Iteration: --- → 52.1 - Oct 3
Keywords: meta
Priority: P3 → P1
Summary: [meta] Enable RDM redesign in Nightly only → Enable RDM redesign in Nightly only
Whiteboard: [multiviewport] [userstory] → [multiviewport] [reserve-rdm]
Comment on attachment 8795000 [details]
Bug 1296498 - Enable new RDM UI for Nightly.

https://reviewboard.mozilla.org/r/81190/#review79838

Glad to see this enabled on Nightly \o/ The patch itself is fine (hence r+), but I'm wondering why can't we just flip the pref to true and let it ride trains, if the plan is to enable it in 52 by default. 

Also, is there a bug filed about removing the old RDM?
Attachment #8795000 - Flags: review?(ntim.bugs) → review+
(In reply to Tim Nguyen :ntim (use needinfo?) from comment #3)
> Comment on attachment 8795000 [details]
> Bug 1296498 - Enable new RDM UI for Nightly.
> 
> https://reviewboard.mozilla.org/r/81190/#review79838
> 
> Glad to see this enabled on Nightly \o/ The patch itself is fine (hence r+),
> but I'm wondering why can't we just flip the pref to true and let it ride
> trains, if the plan is to enable it in 52 by default. 

I'm just being a bit conservative...  I want to make sure we're in a good place with both features and stability before we go ahead and let it ride the trains.  I've filed bug 1305769 to do so when we're ready.

> Also, is there a bug filed about removing the old RDM?

Just filed bug 1305777 for this.
Comment on attachment 8795000 [details]
Bug 1296498 - Enable new RDM UI for Nightly.

https://reviewboard.mozilla.org/r/81190/#review80268

::: devtools/client/styleeditor/test/browser_styleeditor_media_sidebar_links.js:10
(Diff revision 3)
>  "use strict";
>  
>  /* Tests responsive mode links for
>   * @media sidebar width and height related conditions */
>  
> +Services.prefs.setBoolPref("devtools.responsive.html.enabled", false);

It would be nice to have this feature working with the new RDM, can you file a bug/fix it here?
Comment on attachment 8795811 [details]
Bug 1296498 - Update style editor RDM usage for new RDM.

https://reviewboard.mozilla.org/r/81756/#review80414

Thanks for working on it!

I assume you'll need to update the values in browser_styleeditor_media_sidebar.js as well to match the new ones in media-rules.css to fix the failures.

::: devtools/client/styleeditor/test/browser_styleeditor_media_sidebar_links.js:83
(Diff revision 2)
>    is(dimension, value, `${type} should be properly set.`);
>  }
>  
>  function* closeRDM(tab, ui) {
>    info("Closing responsive mode");
> +  // yield new Promise(() => {});

nit: I assume this comment was for debugging, please remove it if it's the case.

::: devtools/client/styleeditor/test/browser_styleeditor_media_sidebar_links.js:117
(Diff revision 2)
> +      }
> +      info(`Got content-resize to a ${type} of ${value}`);
>        resolve();
>      };
> -    info(`Waiting for contentResize to a ${type} of ${value}`);
> -    manager.on("contentResize", onResize);
> +    info(`Waiting for content-resize to a ${type} of ${value}`);
> +    // Old RDM emits on manager

Nothing to do within the code, but you might want to comment on bug 1305777 with a reminder to clean this up.
Attachment #8795811 - Flags: review?(ntim.bugs) → review+
Pushed by jryans@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e943b4729dce
Enable new RDM UI for Nightly. r=ntim
https://hg.mozilla.org/integration/autoland/rev/f6c5daab390c
Update style editor RDM usage for new RDM. r=ntim
https://hg.mozilla.org/mozilla-central/rev/e943b4729dce
https://hg.mozilla.org/mozilla-central/rev/f6c5daab390c
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Depends on: 1311605
No longer depends on: 1311605
Depends on: 1331601
Depends on: 1332908
Depends on: 1357406
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.