Closed Bug 778174 Opened 8 years ago Closed 7 years ago

[responsive mode] add a vertical resizer

Categories

(DevTools :: Responsive Design Mode, defect)

x86
All
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 25

People

(Reporter: paul, Assigned: andreldm1989)

References

Details

Attachments

(7 files, 1 obsolete file)

No description provided.
Blocks: 749628
Hubert, could you take a look at this?
Assignee: nobody → hubert.sablonniere
Hubert, do you still want to work on that?
Yes, I would love to but I'm a bit overwhelmed with my teaching. I know this feature is important. What's the timeframe for FF 20?

I already had a look at how it could be implemented. I think a weekend would do the trick (including tests...) but I won't be able to do it before the 21 dec.

What do we do?
No hurry. No deadline.

> What do we do?

We wait until you have time :)
Why did I ask? Mozilla I love you!!
Component: Developer Tools → Developer Tools: Responsive Mode
Attached patch Patch that solves the bug (obsolete) — Splinter Review
Sorry Hubert, I couldn't wait :D
I just picked this for my first bugfix as it seemed pretty easy.
As I've said, it's my first bugfix here, please evaluate the patch and tell me what I have done wrong :D
PS: I don't know how to send a binary file(png) along with the patch, so in order to this patch work correctly, the file browser/themes/<platform>/devtools/responsive-vertical-resizer.png should be cloned, rotated(90°) and renamed as responsive-horizontal-resizer.png. Sorry for the inconvenience :(
Just in case someone needs the png file.
Attachment #772454 - Flags: review?(paul)
Hey André, no worries. I've been pretty busy these days. Thx for your submission...
Comment on attachment 772454 [details] [diff] [review]
Patch that solves the bug

André, thanks a lot for your help.

To add a file (binary or not), just do "hg add path/to/the/file".

Instead of creating a new file though, I'd suggest that you use responsive-vertical-resizer.png and you apply a CSS transform to it (transform:rotate(90deg)). You might want want to rename this file to "responsive-bar-resizer.png".

You use translate(12,12). You want to use translate(-12,12) (to align horizontally the image).

In the function `onDrag`, you should do: `if (this.ignoreX) deltaX = 0`.
We don't want to change the width with this resizer is used.
Attachment #772454 - Flags: review?(paul)
Paul, thanks for the directions.
I did as you said, but the results of css transform are not nice.
I think it's a good idea to reuse a resource, but in this case there's a deterioration of the image quality.
Check the attachment.
Comment on attachment 772963 [details]
Image vs CSS Transform comparison

This is weird. And it should not happen. Can I ask you to try to reproduce this bug within a web page?
I've setup a small test, please take a look.
Same behavior under Chromium 26.
Interesting. What happens if you add `image-rendering: -moz-crisp-edges`?
The issue is because of odd number of pixels in width. And yes, `image-rendering: -moz-crisp-edges` fixes the issue.
Great, now it looks nice.
What do you think about "rotate(90deg) translate(20px, -12px)",  these values were based on trial and error, do think there's a better way to do it?
Latter I'll prepare another patch.
Once again thanks.
You should use `translate(12px, -12px) ` to match what we do for the other resizer. But start with translate then rotate `translate(12,-12) rotate(90deg)`. I think this will work.
Attached image Rendering bug
Once again something strange happened. I've followed your instructions and `translate(12,-12) rotate(90deg)` doesn't work as it should(see attachment). Then I tried `rotate(90deg) translate(20px, 5px)` and worked just fine under Linux. Then I hacked the onmi.ja under Windows and while dragging the bar it becomes weird(see attachment). At work's machine, I also hacked the onmi.ja, but it seems ok.
Any guess?

My box: Archlinux | Windows 64-bit Ultimate - Core2Duo + NVidia GTX460
Work's box: Windows 64-bit Professional - Phenom II + Radeon HD 4250
Can you attach the patch?
Assignee: hubert.sablonniere → andreldm1989
Attached file Patch css-rotate
This patch uses the css transform rotate. Works, but only on my PC has shown some bugs.
Attachment #772454 - Attachment is obsolete: true
Attached patch Patch new-imageSplinter Review
This patch creates new rotated image. Works fine on every PC I've tested.
Comment on attachment 777560 [details] [diff] [review]
Patch new-image

Let's go for this approach.

Thank you André.
Attachment #777560 - Flags: review+
This orange (bug 866671) is triggered on 4 platforms with this patch. Let's try with a more recent tree: https://tbpl.mozilla.org/?tree=Try&rev=c3646bda31c8
That's better.
Whiteboard: [land-in-fx-team]
https://hg.mozilla.org/integration/fx-team/rev/648ab92d0f63
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/648ab92d0f63
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 25
Attached patch patch-fix-rotateSplinter Review
While testing the nightly, I noticed a problem when resizing with the new resizer and then rotate, it causes the resizer to move to an incorrect position.
Patch attached.
(In reply to André Miranda from comment #29)
> Created attachment 790619 [details] [diff] [review]
> patch-fix-rotate
> 
> While testing the nightly, I noticed a problem when resizing with the new
> resizer and then rotate, it causes the resizer to move to an incorrect
> position.
> Patch attached.

Thank you!
Can I ask you to file a new bug and attach this patch?
Done: bug 906025
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.