The default bug view has changed. See this FAQ.

[Responsive Mode] restore previous size / preset

RESOLVED FIXED in Firefox 15

Status

()

Firefox
Developer Tools
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: paul, Unassigned)

Tracking

Trunk
Firefox 15
x86
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fixed-in-fx-team])

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

5 years ago
We should restore the latest size and preset.
(Reporter)

Comment 1

5 years ago
Created attachment 626043 [details] [diff] [review]
patch v1
(Reporter)

Updated

5 years ago
Attachment #626043 - Flags: review?(dcamp)
(Reporter)

Updated

5 years ago
Blocks: 749628
(Reporter)

Updated

5 years ago
Blocks: 752857
No longer blocks: 749628
(Reporter)

Comment 2

5 years ago
Created attachment 627753 [details] [diff] [review]
patch v1

rebased
(Reporter)

Updated

5 years ago
Attachment #626043 - Attachment is obsolete: true
Attachment #626043 - Flags: review?(dcamp)
(Reporter)

Updated

5 years ago
Attachment #627753 - Flags: review?(dcamp)

Comment 3

5 years ago
Comment on attachment 627753 [details] [diff] [review]
patch v1

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

::: browser/devtools/responsivedesign/responsivedesign.jsm
@@ +76,5 @@
>      this.presets = [{custom: true}];
>    }
>  
>    // Default size. The first preset (custom) is the one that will be used.
>    let bbox = this.stack.getBoundingClientRect();

Do you need this bbox

@@ +81,5 @@
> +
> +  try {
> +    this.presets[0].width = Services.prefs.getIntPref("devtools.responsiveUI.customWidth");
> +    this.presets[0].height = Services.prefs.getIntPref("devtools.responsiveUI.customHeight");
> +    this.currentPreset = Services.prefs.getIntPref("devtools.responsiveUI.currentPreset");

Should there be a sanity check on these values?  Can a really-large custom size cause any real problems?

Should the preset be some sort of absolute string rather than an integer index?  That could make later updates to the preset list less fragile.

@@ +306,5 @@
>     */
>    rotate: function RUI_rotate() {
>      this.setSize(this.currentHeight, this.currentWidth);
> +    if (this.currentPreset == 0)
> +      this.saveCustomSize();

Do you save the rotation value anywhere?
Attachment #627753 - Flags: review?(dcamp)

Comment 4

5 years ago
(In reply to Dave Camp (:dcamp) from comment #3)
> Comment on attachment 627753 [details] [diff] [review]
> patch v1
> 
> Review of attachment 627753 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/devtools/responsivedesign/responsivedesign.jsm
> @@ +76,5 @@
> >      this.presets = [{custom: true}];
> >    }
> >  
> >    // Default size. The first preset (custom) is the one that will be used.
> >    let bbox = this.stack.getBoundingClientRect();
> 
> Do you need this bbox
... outside of the catch block?
(Reporter)

Updated

5 years ago
No longer blocks: 752857
(Reporter)

Comment 5

5 years ago
Created attachment 629500 [details] [diff] [review]
v1.1

addressed dave's comments
(Reporter)

Updated

5 years ago
Attachment #627753 - Attachment is obsolete: true
(Reporter)

Updated

5 years ago
Attachment #629500 - Flags: review?(dcamp)

Updated

5 years ago
Attachment #629500 - Flags: review?(dcamp) → review+

Comment 6

5 years ago
Created attachment 629533 [details] [diff] [review]
un-bitrotted

I had bitrotted you pretty badly, so I fixed it.
Attachment #629500 - Attachment is obsolete: true
(Reporter)

Comment 7

5 years ago
https://hg.mozilla.org/integration/fx-team/rev/ec971b2d6a30
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/ec971b2d6a30
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 15
You need to log in before you can comment on or make changes to this bug.