You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flex.apache.org by Justin Mclean <ju...@classsoftware.com> on 2014/05/17 17:11:17 UTC
Fix for FLEX-369
Hi,
Just checked in a fix that will causes some tests to fail. This fixes a bug that was outstanding since Flex 2.0 days!
Here the tests that fail:
[java] components/Alert/Styles/Alert_Styles Alert_messageStyleName Failed CompareBitmap(body:step 2) compare returned[object BitmapData]
[java] components/Alert/Styles/Alert_Styles Alert_titleStyleName Failed CompareBitmap(body:step 2) compare returned[object BitmapData]
[java] components/Alert/Styles/Alert_Styles Alert_embedded_font_verdana Failed CompareBitmap(body:step 3) compare returned[object BitmapData]
[java] components/Alert/Styles/Alert_Styles Alert_embedded_font_verdana_italic Failed CompareBitmap(body:step 3) compare returned[object BitmapData]
[java] components/Alert/Styles/Alert_Styles Alert_embedded_font_verdana_bold Failed CompareBitmap(body:step 3) compare returned[object BitmapData]
All but one are sub pixel differences and the one failing is doing the wrong thing in allowing the alert text to overflow onto the buttons. Once someone confirms all is good that I'll check in the updated bitmaps.
Thanks,
Justin
Re: Fix for FLEX-369
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> Luck or not, if other folks have controlled the
> size of the Alert (which is allowed) and it "just worked" for them, with
> these changes, it now will not.
Just to be clear this is probably quite uncommon scenario ie setting the hight to be smaller than than needed.
> I'm wondering if you allow that, if these tests will pass and you can still
> clip. Another possible option is to only run the clipping code path if
> there is more than one line of text.
I'll give both it a try.
Justin
Re: Fix for FLEX-369
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> Right now, the changes won't let the gutter and button overlap. I'm
> wondering if you allow that, if these tests will pass and you can still
> clip. Another possible option is to only run the clipping code path if
> there is more than one line of text.
Ended up being slightly more complex as there was some hard coded padding as well as top and bottom padding. I've changed it so at least one line of text will show even if overlaps and if it's big enough you give a nice amount of padding (same as before) and in the normal case the test is position as it was before.
I've checked in the modified code and new baselines.
Thanks,
Justin
Re: Fix for FLEX-369
Posted by Alex Harui <ah...@adobe.com>.
On 5/22/14 9:56 AM, "Justin Mclean" <ju...@classsoftware.com> wrote:
>Hi,
>
>> I think the textfield's gutter needs to be compensated for in this sort
>>of
>> fix. I'm wondering if it is as simple as allowing another 4 pixels in
>> height before clipping so the top and bottom gutters are essentially
>> ignored.
>
>That wouldn't fix the shader test. It's setting the Alert height too
>small to properly contain all the content. Pervious it was just luck that
>the text did not overlap as it has no defenders.
In my results from the Shader test, the Alert is the same size, the button
position appears to be the same, but in the baseline all the text shows,
probably because the gutter of the TextField is under the button, but the
text is fully visible. Luck or not, if other folks have controlled the
size of the Alert (which is allowed) and it "just worked" for them, with
these changes, it now will not.
Right now, the changes won't let the gutter and button overlap. I'm
wondering if you allow that, if these tests will pass and you can still
clip. Another possible option is to only run the clipping code path if
there is more than one line of text.
-Alex
Re: Fix for FLEX-369
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> I think the textfield's gutter needs to be compensated for in this sort of
> fix. I'm wondering if it is as simple as allowing another 4 pixels in
> height before clipping so the top and bottom gutters are essentially
> ignored.
That wouldn't fix the shader test. It's setting the Alert height too small to properly contain all the content. Pervious it was just luck that the text did not overlap as it has no defenders.
Justin
Re: Fix for FLEX-369
Posted by Alex Harui <ah...@adobe.com>.
In the other thread, I noted that the Alert in the Shader test is having
the bottom of the text clipped by the button. Maybe these emails are
still not getting delivered? Anyway, IMO, clipped a single line of text
would probably generate complaints from anyone else who has a similar
setup.
I also noted that clipping text could be risky because important
information could be clipped, so adding a flag to control clipping
behavior might be warranted.
I think the textfield's gutter needs to be compensated for in this sort of
fix. I'm wondering if it is as simple as allowing another 4 pixels in
height before clipping so the top and bottom gutters are essentially
ignored.
-Alex
On 5/22/14 8:52 AM, "Justin Mclean" <ju...@classsoftware.com> wrote:
>Hi,
>
>> Please fix the code or the tests (and any others in the full run that
>>might
>> be broken by this change), or revert.
>
>I am still waiting feedback on this on this change before changing the
>base images. Changing the base images will fix the test.
>
>Thanks,
>Justin
Re: Fix for FLEX-369
Posted by Tom Chiverton <tc...@extravision.com>.
On 22/05/14 16:52, Justin Mclean wrote:
> I am still waiting feedback
I didn't see any here, and there's no discussion on Jira about the
method of fixing.
I see from the diff you basically clamp the height to that available;
this is certainly no worse as long text is cut off already, just not
soon enough.
Tom
Re: Fix for FLEX-369
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> Please fix the code or the tests (and any others in the full run that might
> be broken by this change), or revert.
I am still waiting feedback on this on this change before changing the base images. Changing the base images will fix the test.
Thanks,
Justin
Re: Fix for FLEX-369
Posted by Erik de Bruin <er...@ixsoftware.nl>.
Please fix the code or the tests (and any others in the full run that might
be broken by this change), or revert. I don't want to veto this, but
intentionally breaking Mustella is not a good way to make a point about a
fix...
EdB
On Sat, May 17, 2014 at 5:11 PM, Justin Mclean <ju...@classsoftware.com>wrote:
> Hi,
>
> Just checked in a fix that will causes some tests to fail. This fixes a
> bug that was outstanding since Flex 2.0 days!
>
> Here the tests that fail:
>
> [java] components/Alert/Styles/Alert_Styles Alert_messageStyleName
> Failed CompareBitmap(body:step 2) compare returned[object BitmapData]
> [java] components/Alert/Styles/Alert_Styles Alert_titleStyleName
> Failed CompareBitmap(body:step 2) compare returned[object BitmapData]
> [java] components/Alert/Styles/Alert_Styles
> Alert_embedded_font_verdana Failed CompareBitmap(body:step 3) compare
> returned[object BitmapData]
> [java] components/Alert/Styles/Alert_Styles
> Alert_embedded_font_verdana_italic Failed CompareBitmap(body:step 3)
> compare returned[object BitmapData]
> [java] components/Alert/Styles/Alert_Styles
> Alert_embedded_font_verdana_bold Failed CompareBitmap(body:step 3) compare
> returned[object BitmapData]
>
> All but one are sub pixel differences and the one failing is doing the
> wrong thing in allowing the alert text to overflow onto the buttons. Once
> someone confirms all is good that I'll check in the updated bitmaps.
>
> Thanks,
> Justin
--
Ix Multimedia Software
Jan Luykenstraat 27
3521 VB Utrecht
T. 06-51952295
I. www.ixsoftware.nl