Closed Bug 1873929 Opened 2 years ago Closed 1 year ago

RDM "offline" setting in the throttling menu is not working

Categories

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

defect

Tracking

(firefox136 fixed)

RESOLVED FIXED
136 Branch
Tracking Status
firefox136 --- fixed

People

(Reporter: jdescottes, Assigned: xpresskaran98)

References

Details

Attachments

(2 files)

STRs:

  • open a page
  • enable RDM
  • set Offline in RDM's throttling menu

ER: Page should be offline
AR: Nothing happens

The RDM implementation can be found at https://searchfox.org/mozilla-central/rev/52f45d12530f9f8da61c6c7237003b5d1499b6d1/devtools/client/responsive/ui.js#856 . There is no handling of offline.

The netmonitor implementation is at https://searchfox.org/mozilla-central/rev/52f45d12530f9f8da61c6c7237003b5d1499b6d1/devtools/client/netmonitor/src/connector/index.js#512. The offline setting is handled via target configuration, eg:

      await this.commands.targetConfigurationCommand.updateConfiguration({
        setTabOffline: id === throttlingProfiles.PROFILE_CONSTANTS.OFFLINE,
      });

It would really be nice to have a shared command to handle throttling and use this from both RDM and netmonitor, to avoid this duplicated code and this kind of bugs.

See Also: → 1872381
Severity: -- → S3
Priority: -- → P2

Hello,

I noticed this issue has been open for a while, and I would like to work on it. Could you please assign it to me?

(In reply to Karan Yadav from comment #1)

Hello,

I noticed this issue has been open for a while, and I would like to work on it. Could you please assign it to me?

Thanks for your interest! Yes it would be great to get some traction here.

I mentioned adding a shared command to handle this in the summary, but if it's your first bug here, I think for now just fixing the issue is enough and we can file a follow up in order to reduce the code duplication.

So I would say feel free to duplicate the logic found in the netmonitor over to responsive/ui.js for now.

But it would be great to add a test for this, maybe starting based on devtools/client/responsive/test/browser/browser_network_throttling.js and reusing the offline testing logic from devtools/client/netmonitor/test/browser_net_offline_mode.js

Regarding bug assignment, the bug will be automatically assigned to you when you submit a patch on phabricator. Our getting started docs can be found at https://firefox-source-docs.mozilla.org/setup/index.html

Thanks for the guidance! I’ll work on the patch and push it to Phabricator soon. In case I get stuck, where would be the best place to ask for help?

(In reply to Karan Yadav from comment #3)

Thanks for the guidance! I’ll work on the patch and push it to Phabricator soon. In case I get stuck, where would be the best place to ask for help?

You can ask here in Bugzilla, or in Phabricator once you pushed a patch, or on Element if you want to chat: https://chat.mozilla.org/#/room/#devtools:mozilla.org (although most of the team will have limited availability until the beginning of January :) )

Assignee: nobody → xpresskaran98
Status: NEW → ASSIGNED

Hey there,
I have written the patch. If there are any issues or suggestions for improvement, feel free to let me know. I'm happy to make the necessary changes.
Thanks!

Hello,
I have done the changes as discussed here: https://phabricator.services.mozilla.com/D232195.

Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/39387a6663de [devtools] Set the offline configuration to True if id matches with the PROFILE_CONSTANTS.OFFLINE id. r=devtools-reviewers,jdescottes https://hg.mozilla.org/integration/autoland/rev/bca5099b60a8 [devtools] Added the test for browser offline mode in RDM. r=devtools-reviewers,jdescottes
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 136 Branch
You need to log in before you can comment on or make changes to this bug.