Enable RDM redesign in Nightly only

RESOLVED FIXED in Firefox 52

Status

defect
P1
normal
RESOLVED FIXED
3 years ago
Last year

People

(Reporter: jryans, Assigned: jryans)

Tracking

unspecified
Firefox 52
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox52 fixed)

Details

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

Attachments

(2 attachments)

Assignee

Description

3 years ago
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.
Assignee

Updated

3 years ago
Depends on: 1240912
Flags: qe-verify-
Whiteboard: [multiviewport] [userstory]
Assignee

Updated

3 years ago
Depends on: 1300214
Assignee

Updated

3 years ago
No longer depends on: 1273997
Assignee

Updated

3 years ago
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Assignee

Updated

3 years ago
No longer depends on: 1249295
Assignee

Comment 1

3 years ago
I think we're now far enough along to flip on new RDM for Nightly only here.
Comment hidden (mozreview-request)
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 3

3 years ago
mozreview-review
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+
Comment hidden (mozreview-request)
Assignee

Comment 7

3 years ago
(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 hidden (mozreview-request)

Comment 10

3 years ago
mozreview-review
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 hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 16

3 years ago
mozreview-review
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+
Comment hidden (mozreview-request)

Comment 19

3 years ago
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

Comment 20

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e943b4729dce
https://hg.mozilla.org/mozilla-central/rev/f6c5daab390c
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Depends on: 1311605

Updated

3 years ago
No longer depends on: 1311605

Updated

2 years ago
Depends on: 1331601

Updated

2 years ago
Depends on: 1332908

Updated

2 years ago
Depends on: 1357406
Assignee

Updated

2 years ago
No longer depends on: 1357406

Updated

Last year
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.