If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Improve logic behind setting state of Constrain checkbox in Image Properties dialog

VERIFIED FIXED in mozilla0.9.9

Status

SeaMonkey
Composer
P3
normal
VERIFIED FIXED
17 years ago
13 years ago

People

(Reporter: sujay, Assigned: Charles Manske)

Tracking

Trunk
mozilla0.9.9

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

17 years ago
using 10/26 build of netscape

1) launch netscape
2) launch composer
3) insert any image
4) go into image dialog
5) click on Custom Size
6) uncheck Constrain option
7) click OK on image dialog panel
8) now go back into the image dialog 

notice the constrain checkbox is checked again.

Comment 1

17 years ago
accepting bug though I don't know that we really want to keep track of whether
the constrain checkbox is checked (for each image? last time in image dialog?)
It would seem like most users would want to constrain most of the time so always
checking the checkbox would seem to be the best thing to do.
Status: NEW → ASSIGNED
Summary: contrain checkbox keeps getting checked when going back into image dialog → constrain checkbox keeps getting checked when going back into image dialog

Comment 2

17 years ago
hand this bug over to Charley.
I'm not sure if we should do anything about this "bug" or not.  I think the 4.x
behavior was the opposite of this bug (at least on Macintosh); we seem to never
check the constrain checkbox (no matter what state it was last left in).

I think the current behavior in NS6 is desirable and I'm not sure we should
address this bug (maybe make it a P5 bug?).
Assignee: brade → cmanske
Status: ASSIGNED → NEW
(Assignee)

Comment 3

17 years ago
I'm inclined to keep the default as "constrain" with every new launch of the 
image dialog. I think having constrain on is the most desired state, so 
remembering the previous dialog state doesn't seem the best thing to do.
Target Milestone: --- → mozilla0.9
(Assignee)

Comment 4

17 years ago
Anyone who feels strongly about changing current behavior can reopen this bug.
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → WONTFIX
(Reporter)

Comment 5

17 years ago
verified in 11/8 build.
Status: RESOLVED → VERIFIED

Comment 6

16 years ago
I'm feeling inclined to reopen this bug. Though the default state of Constrain
should be checked for a given image, once the user unchecks it that action
should persist.

Let's say I decided I want an image to be 100x10 pixels (not its' intrinsic
dimensions). I then want to alter the width a few times to make it match
something a few pixels wider. If I reopen the image properties dialog and change
the width to 101 pixels, then since Constrain is now rechecked (against my
wishes) the height jumps to 68 pixels. Hardly optimal behavior.

As a comparison, Photoshop remembers the Constrain setting on the image across
dialog invocations.
Status: VERIFIED → REOPENED
Resolution: WONTFIX → ---

Comment 7

16 years ago
it'd be nice but this isn't a high priority for us right now
Keywords: helpwanted
Target Milestone: mozilla0.9 → Future

Comment 8

16 years ago
I just recently ran across this bug and thought it was a little annoying.  I was
using a clear pixel gif to help with the layout of my page and had to keep
unchecking the constrain box in order to resize the image.  After having to do
this quite a few times with the same image I found it to be a little frustrating.
(Assignee)

Comment 9

16 years ago
Do you expect the "constrain" checkbox state to be remembered for each image
you use? It would be easy to persist the state so the next time you use the 
dialog, it would be the same, but as soon as you change it when editing 
properties for a different image, that would become the new default.
Would that be OK?

Comment 10

16 years ago
In truth, yes, it should be remembered on a per-image basis.

Now, I realize this will be impossible across document opens, since the contrain
state won't be saved. What we could do in that instance is check image
dimensions at open, and if they're proportional, then note it as constrained,
but if they're not, then note that it isn't.
(Assignee)

Comment 11

16 years ago
Interesting suggestion, since we should know the "original" dimensions of the 
image (unless we can't load the image, such as when user is offline) and we can 
therefore compare those numbers to the html attributes.
I'll look into it later.
Status: REOPENED → ASSIGNED
Target Milestone: Future → mozilla1.0

Comment 12

16 years ago
The original dimensions of the image aren't necessarily important. It's the
dimensions in the HTML or CSS code that are important, since they can override
the intrinsic dimensions.
(Assignee)

Comment 13

16 years ago
I didn't explain fully: The original dimensions are important to compare their
ratio to the ratio of the HTML attributes. If the same, I'd probably check the
constrain checkbox, if different, I wouldn't check it.

Comment 14

16 years ago
spam composer change
Component: Editor: Core → Editor: Composer

Comment 15

16 years ago
Got it, Charles.
(Assignee)

Updated

16 years ago
Whiteboard: EDITORBASE (1 day)
(Assignee)

Comment 16

16 years ago
load balancing
Target Milestone: mozilla1.0 → mozilla0.9.9

Updated

16 years ago
Whiteboard: EDITORBASE (1 day) → EDITORBASE- (1 day)
(Assignee)

Updated

16 years ago
Whiteboard: EDITORBASE- (1 day) → (1 day)
(Assignee)

Comment 17

16 years ago
Fix for this is ready, just needs a bit more testing. Lower priority, so pushing
off that task.
Keywords: helpwanted → nsbeta1+

Comment 18

16 years ago
Looking forward to testing it. What'd you do, check proportionality at document
open or at each Image dialog invocation?
(Assignee)

Comment 19

16 years ago
I can't attach a patch until I checkin other fixes in the image dialog files, but
all I do is compare the ratio of the actual height/width to the height/width in
the input fields. This is only done if "Custom" checkbox has been checked and 
height or width has been changed. This test is done when any image is loaded into
the dialog.

Comment 20

16 years ago
Are there other bugs this one should depend on, then?
(Assignee)

Comment 21

16 years ago
Created attachment 68549 [details] [diff] [review]
Fix

Primary fix was very simple: move setting of radio into SetSizeRadio().
I also figure out how Constrain checkbox should be checked base on ratio of
real height/width as compared to users.
Most of the change is substituting "actual..." with "gActual..." for variable 
names.
(Assignee)

Updated

16 years ago
Whiteboard: (1 day) → FIX IN HAND need r=,sr=
(Assignee)

Comment 22

16 years ago
Greg: not dependencies, just multiple bug fixes in same file.

Comment 23

16 years ago
Comment on attachment 68549 [details] [diff] [review]
Fix

You can't assume that the proportions are goint to be exact. If gActualHeight
== gActualWidth then that's not a problem, otherwise if gActualHeight >
gActualWidth you need to calculate Math.round( gActualWidth * height /
gActualHeight ) == width and vice versa. Otherwise the patch looks fine.
(Assignee)

Comment 24

16 years ago
For more accurate ratio calculations, I'll use:
+function  SetSizeRadio(width, height)
+{
+  if (!(width || height) || (gActualWidth && gActualHeight && width ==
gActualWidth && height == gActualHeight))
+    gDialog.actualSizeRadio.radioGroup.selectedItem = gDialog.actualSizeRadio;
+
+  if (!gDialog.actualSizeRadio.selected)
+  {
+    gDialog.actualSizeRadio.radioGroup.selectedItem = gDialog.customSizeRadio;
+    // Decide if user's sizes are in the same ratio as actual sizes
+    if (gActualWidth && gActualHeight)
+    {
+      if (gActualWidth > gActualHeight)
+        gDialog.constrainCheckbox.checked = (Math.round(gActualHeight * width /
gActualWidth) == height);
+      else
+        gDialog.constrainCheckbox.checked = (Math.round(gActualWidth * height /
gActualHeight) == width);
+    }
+}
+
(Assignee)

Comment 25

16 years ago
Comment on attachment 68549 [details] [diff] [review]
Fix

r=neil@parkwaycc.co.uk
(from discussion on IRC)
Attachment #68549 - Flags: review+

Comment 26

16 years ago
Editing Summary to better reflect the problem and solution.
Summary: constrain checkbox keeps getting checked when going back into image dialog → Improve logic behind setting state of Constrain checkbox in Image Properties dialog

Comment 27

16 years ago
Comment on attachment 68549 [details] [diff] [review]
Fix

sr=kin@netscape.com

I noticed that some dump statements were added. Are those to debug work in
progress? Will they be removed at a later point?
Attachment #68549 - Flags: superreview+

Updated

16 years ago
Whiteboard: FIX IN HAND need r=,sr= → FIX IN HAND, reviewed
(Assignee)

Comment 28

16 years ago
Created attachment 68588 [details] [diff] [review]
The right files changed!

Sorry! that patch had lots of irrelevant files. This attachment is exactly the
same as what was reviewed, only all of the other file diffs were deleted.
Attachment #68549 - Attachment is obsolete: true
(Assignee)

Comment 29

16 years ago
checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago16 years ago
Resolution: --- → FIXED
Whiteboard: FIX IN HAND, reviewed

Comment 30

16 years ago
So this should show up in tomorrow's nightly?

Comment 31

16 years ago
Although the code actually checked in is the correct code that I reviewed,
neither of these attachments appears to match up :-)
(Reporter)

Updated

16 years ago
Whiteboard: cannot verify this until 123727 is fixed.

Updated

16 years ago
Depends on: 123727

Comment 32

16 years ago
Sujay, you can verify this by double-clicking or contextually-clicking the image.
No longer depends on: 123727

Updated

16 years ago
Depends on: 123727
(Reporter)

Comment 33

16 years ago
still not fixed in 2/21 trunk build....that checkbox always gets checked even
though
you uncheck it....
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Updated

16 years ago
No longer depends on: 123727

Comment 34

16 years ago
You sure, Sujay? It works for me using Mac/2002022108. Constrain checkbox state
is set as appropriate.

Comment 35

16 years ago
That is, Sujay, it should be checked even if you uncheck it, as long as you
didn't change the dimensions.

Since there's no way to store checkbox state with the saved document, this is
the only way that makes sense; that is, to detect contrainedness at Image
Properties dialog invocation.
(Reporter)

Comment 36

16 years ago
okay it works now...
Status: REOPENED → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED
(Reporter)

Comment 37

16 years ago
v
Status: RESOLVED → VERIFIED

Updated

16 years ago
Whiteboard: cannot verify this until 123727 is fixed.
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.