Closed Bug 778174 Opened 8 years ago Closed 7 years ago
[responsive mode] add a vertical resizer
256 bytes, image/png
63.28 KB, image/png
1.39 KB, application/octet-stream
38.03 KB, image/png
17.37 KB, text/plain
16.28 KB, patch
|Details | Diff | Splinter Review|
997 bytes, patch
|Details | Diff | Splinter Review|
No description provided.
Hubert, could you take a look at this?
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
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.
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.
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.
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?
This patch uses the css transform rotate. Works, but only on my PC has shown some bugs.
Attachment #772454 - Attachment is obsolete: true
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
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
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
You need to log in before you can comment on or make changes to this bug.