778174 - [responsive mode] add a vertical resizer

Status: RESOLVED FIXED

(DevTools :: Responsive Design Mode, defect)

Comment 1

[Paul Rouget [:paul]]

Hubert, could you take a look at this?

Comment 2

[Paul Rouget [:paul]]

Hubert, do you still want to work on that?

Comment 3

[hsablonniere]

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?

Comment 4

[Paul Rouget [:paul]]

No hurry. No deadline.

> What do we do?

We wait until you have time :)

Comment 5

[hsablonniere]

Why did I ask? Mozilla I love you!!

Comment 6

[André Miranda]

Attachment: Patch that solves the bug

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 :(

Comment 7

[André Miranda]

Attachment: Resource necessary for the patch

Just in case someone needs the png file.

Comment 8

[hsablonniere]

Hey André, no worries. I've been pretty busy these days. Thx for your submission...

Comment 9

[Paul Rouget [:paul]]

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.

Comment 10

[André Miranda]

Attachment: Image vs CSS Transform comparison

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 11

[Paul Rouget [:paul]]

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?

Comment 12

[André Miranda]

Attachment: Testing blur of transform-rotate.

I've setup a small test, please take a look.
Same behavior under Chromium 26.

Comment 13

[Paul Rouget [:paul]]

Interesting. What happens if you add `image-rendering: -moz-crisp-edges`?

Comment 14

[Girish Sharma [:Optimizer]]

The issue is because of odd number of pixels in width. And yes, `image-rendering: -moz-crisp-edges` fixes the issue.

Comment 15

[André Miranda]

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.

Comment 16

[Paul Rouget [:paul]]

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.

Comment 17

[André Miranda]

Attachment: 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

Comment 18

[Paul Rouget [:paul]]

Can you attach the patch?

Comment 19

[André Miranda]

Attachment: Patch css-rotate

This patch uses the css transform rotate. Works, but only on my PC has shown some bugs.

Comment 20

[André Miranda]

Attachment: Patch new-image

This patch creates new rotated image. Works fine on every PC I've tested.

Comment 21

[Paul Rouget [:paul]]

Comment on attachment 777560 [details] [diff] [review]
Patch new-image

Let's go for this approach.

Thank you André.

Comment 22

[Paul Rouget [:paul]]

https://tbpl.mozilla.org/?tree=Try&rev=964d12259d90

Comment 23

[Paul Rouget [:paul]]

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

Comment 24

[Paul Rouget [:paul]]

That's better.

Comment 25

[Paul Rouget [:paul]]

https://hg.mozilla.org/integration/fx-team/rev/648ab92d0f63

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/648ab92d0f63

Comment 27

[Paul Rouget [:paul]]

s/12px/-12px/ https://hg.mozilla.org/integration/fx-team/rev/eb7ff2beda97

Comment 28

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/eb7ff2beda97

Comment 29

[André Miranda]

Attachment: 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.

Comment 30

[Paul Rouget [:paul]]

(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?

Comment 31

[André Miranda]

Done: bug 906025