Fix #898 | Added center align css interaction between IE11 and other …

[fortunatomaldonado]

…browsers

Issue

#898
The issue is the difference in center alignment for IE11 and other browsers will clash and produce unexpected behaviors such as unable to unalign. The left and right align work correctly since this is only adding float: left and float: right to the content.

Center align is problematic since in IE11, it adds text-align: center style to the parentNode of the image, image container. While in other browsers the styles margin-right: auto, margin-left: auto and display:block is added to the image.

Solution

A solution is to adjust these styles so that they will be correct no matter which browser is being used.
When aligning in other browsers, the first check getImageAlignment will look for a text-align Style in the image container. If it is found, then that means it has been previously edited and expect to be center aligned. Then it will add the center alignment in the image. The removeImageAlignment will check and remove the text-align style from the image container. setImageAlignment will add the style to the image container so that if the content is being worked on in IE11, it will be properly aligned.

In IE11, the style is removed from the image since they are not needed on the image but on the image container.

I tested on Firefox, Chrome, and IE11.

Let me know if there are any questions or comments about this.
Thank you.

[julien]

Just started reviewing :)

Sent from GH.

[julien]

Hi @fortunatomaldonado,

Thanks for looking into this.

I have several questions:

• Did you notice the tests are failing on travis? Could you have a look into that please?

• I tried to reproduce the bug described in the LPS (without updating alloy-editor) and noticed that if you toggle the "left" or "right" alignment buttons, the style (<p style="text-align:center"></p>) applied to the <p> element surrounding the image is removed, meaning that when you publish again the image is not "centered" anymore ... so I'm wondering if this is really a bug.

[duracell80]

I can help add more information.

On IE only center alignment (via text align) is set in the P element. Left and right is set on the IMG and uses float.

The center text alignment can cause users left aligned text to go center when removing the image though that's not always a reliable reproduction.

However there is a worse issue evident in the pasted code here. The CSS is missing a semi colon, causing styling issues too.

[julien]

Hi @duracell80,

Thanks for adding that.

However there is a worse issue evident in the pasted code here. The CSS is missing a semi colon, causing styling issues too

Which pasted code are we talking about?

[duracell80]

style="text-align:center"

Img Float: left
Img Float: right

Also missing semi colons.

[julien]

Hi again @duracell80,

I'm not seeing that in the alloy-editor demo, but rather something like this

In Chrome

<img data-cke-saved-src="http://21stcenturywaves.com/wp-content/uploads/2009/07/fullmoon.thumbnail.jpg" src="http://21stcenturywaves.com/wp-content/uploads/2009/07/fullmoon.thumbnail.jpg" class="" style="float: left;">

And in IE 11

<p><span tabindex="-1" class="cke_widget_wrapper cke_widget_inline cke_widget_image cke_image_nocaption cke_widget_selected cke_widget_focused" role="region" style="float: left;" contenteditable="false" aria-label=" undefined widget" data-cke-display-name="img" data-cke-filter="off" data-cke-widget-wrapper="1" data-cke-widget-id="0"><img class="cke_widget_element" alt="" src="http://21stcenturywaves.com/wp-content/uploads/2009/07/fullmoon.thumbnail.jpg" data-cke-saved-src="http://21stcenturywaves.com/wp-content/uploads/2009/07/fullmoon.thumbnail.jpg" data-widget="image" data-cke-widget-keep-attr="0" data-cke-widget-upcasted="1" data-cke-widget-data="%7B%22hasCaption%22%3Afalse%2C%22src%22%3A%22http%3A%2F%2F21stcenturywaves.com%2Fwp-content%2Fuploads%2F2009%2F07%2Ffullmoon.thumbnail.jpg%22%2C%22alt%22%3A%22%22%2C%22width%22%3A%22%22%2C%22height%22%3A%22%22%2C%22lock%22%3Atrue%2C%22align%22%3A%22left%22%2C%22classes%22%3Anull%7D"><span class="cke_reset cke_widget_drag_handler_container" style='background: url("http://192.168.1.74:8000/dist/alloy-editor/plugins/widget/images/handle.png") rgba(220, 220, 220, 0.5); left: 0px; top: -15px; display: block;'><img width="15" height="15" title="Click and drag to move" class="cke_reset cke_widget_drag_handler" role="presentation" draggable="true" src="data:image/gif;base64,R0lGODlhAQABAPABAP///wAAACH5BAEKAAAALAAAAAABAAEAAAICRAEAOw==" data-cke-widget-drag-handler="1"></span><span class="cke_image_resizer">​</span></span></p>

So while the float: left; seems to be applied to different elements, I do not see missing semi-colons. Are we talking about the same thing?

Thanks

[fortunatomaldonado]

Hello @julien,

Thank you for looking into this. This is an odd issue and I will try to explain the situation better so we can all be on the same page.

1. I did notice the test failing on travis. I ran the test locally on the latest master a196faf and found the same error logs.

Chrome 70.0.3538 (Windows 10 0.0.0) AutoLink should create a link when pressing ENTER FAILED >AssertionError: expected <p>link www.liferay.com</p><p>text</p> to equal <p>link <a >href="http://www.liferay.com">www.liferay.com</a></p><p>text</p>
at Context.testLink (test/plugins/test/autolink.js:230:20)
at Context. (test/plugins/test/autolink.js:80:22)

I do not believe my changes caused this error. I believe this was the only error but I could be mistaken.

2. Referring to reproducing the LPS bug, working on IE11 centering an image and then using the left or right align does remove the <p style="text-align:center"></p> as you stated. This is not the issue and I will update the LPS to explain the situation. The issue lies in centering an image in IE11 and saving the blog which has the <p style="text-align:center"></p>. Now if you go to Chrome, and open the saved blog, you will see that the image is still centered. If you left or right align, the image will do so and seem like all is fine. When you try to remove the alignment all together, the image will remain centered aligned. Clicking center align will only add and remove the style from the image, but the <p style="text-align:center"></p> will always remain. This is the issue I am trying to resolve.

Let me know if this makes sense and if any more information is needed.
Thank you again.

[duracell80]

I'm looking from a slightly different perspective in published content on DXP 7.0. if you run the same tests in DXP you'll see IE is publishing code without semi colon's which will cause issues in general.

[duracell80]

Can this fix be reviewed please because center alignment should not be being set on the paragraph element, the alignment should apply to the image element.

[julien]

Just started reviewing :)

Sent from GH.

[julien]

Hey @fortunatomaldonado,

I tested this fix in portal regarding #898 and it solves the issue.

Concerning the tests, I'm not sure about what's going on, I've tested master, develop and this branch and I'm not getting the same errors (which indeed do not seem related to your changes).

I'll try running the on the CI again and see if it's still happening.

Thanks!

[fortunatomaldonado]

Hello @julien,

I reran the tests locally on master and I am no longer getting the errors. I am not sure what may have caused it or what made it go away. Let me know if the errors still appears after running CI again.

Thank you.

[julien]

Hi @fortunatomaldonado,

I spent some time changing the saucelabs configuration and fixing the failing test, as soon as this is solved I'll have another look and rerun the tests on travis+saucelabs and keep you updated.

Sorry for the delay

[fortunatomaldonado]

Hello @julien,

No worries. It looks like your changes fixed the issue.

Thank you for your efforts.