[Admin] Do not allow HTML tags for the Product Attribute labels on save by vasilii-b · Pull Request #27371 · magento/magento2

vasilii-b · 2020-03-20T12:52:56Z

Description (*)

This PR adds changes to prevent the store owner to create product attributes which labels contain HTML tags.

HTML tags can be added to the following fields:

Attribute Default Label
Manage Labels Tab - attribute labels for each store separately

ℹ️ It doesn't make sense to allow HTML tags since those are escaped at the view level.
E.g.

magento2/app/code/Magento/LayeredNavigation/view/frontend/templates/layer/view.phtml

Line 37 in ac3eb00

    
           <dt role="heading" aria-level="3" class="filter-options-title"><?= $block->escapeHtml(__($filter->getName())) ?></dt>

Desired result

At the moment the store owner tries to save the attribute, the labels validation must happen and error messages shown to the user that HTML tags are not allowed.

Related Pull Requests

N/A

Fixed Issues (if relevant)

N/A

Manual testing scenarios (*)

Without the fix

Log in in the admin area
Go to Stores -> Attributes -> Products
Open an attribute to edit or create a new one
In the "Default label" wite some HTML tags. E.g <span>
In the Manage Labels Tab add the same HTML tag as above in the "Default Store View" field.
Press the "Save attribute" or "Save and Continue Edit" button
See the product has been saved without any warning

With the fix

Log in in the admin area
Go to Stores -> Attributes -> Products
Open an attribute to edit or create a new one
In the "Default label" wite some HTML tags. E.g <span>
In the Manage Labels Tab add the same HTML tag as above in the "Default Store View" field.
Press the "Save attribute" or "Save and Continue Edit" button
See the product has NOT been saved and there are warnings

Questions or comments

MFTF tests are on the way

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds are green)

eduard13

Hi @vasilii-b, thank you for your contribution, could you please cover the following improvement by an MFTF test and fix the failing Static Tests?

Thank you.

…n labels

vasilii-b · 2020-03-21T16:30:43Z

Hi @eduard13,
I've added the MFTF test and the fixes for the failed static checks. Waiting now for the next round of tests to finish.
Thank you!

eduard13

Hi @vasilii-b, thank you for the MFTF coverage, please check the bellow comments. Feel free to reach me if any questions arise.
Thank you.

eduard13 · 2020-03-21T19:27:55Z

...Magento/Catalog/Test/Mftf/ActionGroup/SaveProductAttributeWithHtmlTagsInLabelActionGroup.xml

+
+<actionGroups xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+              xsi:noNamespaceSchemaLocation="urn:magento:mftf:Test/etc/actionGroupSchema.xsd">
+    <actionGroup name="SaveProductAttributeWithHtmlTagsInLabelActionGroup">


Could you please split this Action group into 2 separated ones, trying to make Action Groups a much atomic as possible?

AdminSaveProductAttributeActionGroup - so it may be used later in other validation scopes

AssertSeeProductAttributeValidationErrorActionGroup - will validate the presence of the error.

Please check the Best Practices.

eduard13 · 2020-03-21T19:37:09Z

app/code/Magento/Catalog/Test/Mftf/Test/AdminProductAttributeLabelDontAllowHtmlTagsTest.xml

+        <actionGroup ref="AdminOpenProductAttributePageActionGroup" stepKey="openProductAttributePage"/>
+        <click selector="{{AdminProductAttributeGridSection.createNewAttributeBtn}}" stepKey="createNewAttribute"/>


I suggest replacing these steps by a separate ActionGroup, that will navigate to NewProductAttributePage.
AdminNavigateToNewProductAttributePageActionGroup, using this page URL ProductAttributePage.

eduard13 · 2020-03-21T19:51:12Z

app/code/Magento/Catalog/Test/Mftf/Section/AdminCreateProductAttributeSection.xml

        <element name="visibleOnCatalogPagesOnStorefront" type="select" selector="#is_visible_on_front"/>
    </section>
+    <section name="ManageLabelsSection">
+        <element name="PageTitle" type="text" selector="//span[text()='Manage Titles (Size, Color, etc.)']" />


I think this element is unnecessary as far as we don't use it.

eduard13 · 2020-03-21T19:53:33Z

app/code/Magento/Catalog/Test/Mftf/Section/AdminCreateProductAttributeSection.xml

        <element name="StorefrontPropertiesSectionToggle" type="button" selector="#front_fieldset-wrapper"/>
        <element name="visibleOnCatalogPagesOnStorefront" type="select" selector="#is_visible_on_front"/>
    </section>
+    <section name="ManageLabelsSection">


Could you please create a separate Section file, and include it in ProductAttributePage page?

eduard13 · 2020-03-21T19:55:18Z

app/code/Magento/Catalog/Test/Mftf/Test/AdminProductAttributeLabelDontAllowHtmlTagsTest.xml

+        </annotations>
+        <before>
+            <!-- Login as admin -->
+            <actionGroup ref="LoginAsAdmin" stepKey="loginAsAdmin"/>


LoginAsAdmin is deprecated, please use AdminLoginActionGroup instead.

eduard13 · 2020-03-21T19:56:09Z

app/code/Magento/Catalog/Test/Mftf/Test/AdminProductAttributeLabelDontAllowHtmlTagsTest.xml

+            <actionGroup ref="AdminLogoutActionGroup" stepKey="logout"/>
+        </after>
+
+        <!-- Go to Stores > Attributes > Product , click "Add New Attribute" -->


The comments are unnecessary, as the stepKeys should be self explanatory.

…o admin-area-product-attribute-can-save-with-html-tags-in-labels

vasilii-b · 2020-03-22T09:28:45Z

Thank you very much for the feedback and the suggestions, @eduard13!
The MFTF tests have been updated ✅

eduard13 · 2020-03-23T07:39:28Z

@magento give me test instance

magento-engcom-team · 2020-03-23T07:39:33Z

Hi @eduard13. Thank you for your request. I'm working on Magento instance for you

magento-engcom-team · 2020-03-23T07:40:19Z

Hi @eduard13, here is your new Magento instance.
Admin access: https://pr-27371.instances.magento-community.engineering/admin_fc49
Login: 680e91f3 Password: fa15dd10e1f7
Instance will be terminated in up to 3 hours.

…o admin-area-product-attribute-can-save-with-html-tags-in-labels

eduard13

Thank you for the adjustments.
The failing tests aren't related to these changes/

engcom-Echo · 2020-03-24T18:07:15Z

@magento run all tests

engcom-Echo · 2020-03-25T07:29:05Z

@magento run all tests

engcom-Echo · 2020-03-26T11:51:38Z

Failed tests not related to the changes in this PR

slavvka

Please resolve the conflict before proceeding with it

Pull Request state was updated. Re-review required.

…ibute-can-save-with-html-tags-in-labels # Conflicts # app/code/Magento/Catalog/Test/Mftf/Section/AdminCreateProductAttributeSection.xml

eduard13

Thank you @vasilii-b.

magento-engcom-team · 2020-03-28T10:39:43Z

Hi @eduard13, thank you for the review.
ENGCOM-7185 has been created to process this Pull Request

slavvka · 2020-03-28T21:10:40Z

@magento run Functional Tests CE, Functional Tests EE, Functional Tests B2B

magento-engcom-team · 2020-03-29T00:17:25Z

Hi @slavvka, thank you for the review.
ENGCOM-7185 has been created to process this Pull Request

… labels on save #27371

m2-assistant · 2020-03-29T02:07:53Z

Hi @vasilii-b, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

product attribute - deny HTML tags for labels

7600c7c

ghost added the Progress: pending review label Mar 20, 2020

magento-engcom-team added Area: Frontend Component: Catalog Component: Eav Release Line: 2.4 Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner labels Mar 20, 2020

eduard13 self-assigned this Mar 20, 2020

fix code style failed tests

6cbfa77

eduard13 requested changes Mar 20, 2020

View reviewed changes

ghost added Progress: needs update and removed Progress: pending review labels Mar 20, 2020

Burlacu Vasilii added 2 commits March 21, 2020 17:06

fix code style failed tests

b4b22a4

Added the MFTF test that checks attribute won't save with HTML tags i…

2e3128f

…n labels

eduard13 requested changes Mar 21, 2020

View reviewed changes

Vasilii Burlacu added 2 commits March 22, 2020 10:42

Merge branch '2.4-develop' of https://github.com/magento/magento2 int…

03e3efb

…o admin-area-product-attribute-can-save-with-html-tags-in-labels

Refactored the MFTF tests according to the review

ae2038e

Vasilii Burlacu added 4 commits March 23, 2020 10:39

Merge branch '2.4-develop' of https://github.com/magento/magento2 int…

ad0844e

…o admin-area-product-attribute-can-save-with-html-tags-in-labels

improved the MFTF tests

32b6ce6

refactored MFTF tests

196e353

refactored MFTF tests

6b6a2f7

eduard13 previously approved these changes Mar 23, 2020

View reviewed changes

ghost added Progress: ready for testing and removed Progress: needs update labels Mar 23, 2020

engcom-Echo self-assigned this Mar 24, 2020

vasilii-b mentioned this pull request Mar 24, 2020

[Admin][improvement] Do not allow HTML tags for product attribute options when type is dropdown or multiple select #27417

Closed

5 tasks

ghost added Progress: accept and removed Progress: extended testing labels Mar 26, 2020

slavvka requested changes Mar 27, 2020

View reviewed changes

ghost assigned slavvka Mar 27, 2020

ghost added Progress: needs update and removed Progress: accept labels Mar 27, 2020

merge remote tracking origin/2.4-develop into admin-area-product-attr…

8cdcbbd

…ibute-can-save-with-html-tags-in-labels # Conflicts # app/code/Magento/Catalog/Test/Mftf/Section/AdminCreateProductAttributeSection.xml

vasilii-b requested a review from eduard13 March 28, 2020 10:35

eduard13 approved these changes Mar 28, 2020

View reviewed changes

ghost added Progress: ready for testing and removed Progress: needs update labels Mar 28, 2020

slavvka approved these changes Mar 29, 2020

View reviewed changes

ghost added Progress: testing in progress Progress: accept and removed Progress: ready for testing Progress: testing in progress labels Mar 29, 2020

magento-engcom-team pushed a commit that referenced this pull request Mar 29, 2020

ENGCOM-7185: [Admin] Do not allow HTML tags for the Product Attribute…

4f8da50

… labels on save #27371

magento-engcom-team merged commit e9994f8 into magento:2.4-develop Mar 29, 2020

vasilii-b deleted the admin-area-product-attribute-can-save-with-html-tags-in-labels branch March 29, 2020 11:01

m2-assistant bot mentioned this pull request Aug 17, 2020

[Issue] [Admin][improvement] Do not allow HTML tags for product attribute options when type is dropdown or multiple select #29602

Closed

4 tasks

		<actionGroup ref="AdminOpenProductAttributePageActionGroup" stepKey="openProductAttributePage"/>
		<click selector="{{AdminProductAttributeGridSection.createNewAttributeBtn}}" stepKey="createNewAttribute"/>

Conversation

vasilii-b commented Mar 20, 2020

Description (*)

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

Uh oh!

eduard13 left a comment

Choose a reason for hiding this comment

Uh oh!

vasilii-b commented Mar 21, 2020

Uh oh!

eduard13 left a comment

Choose a reason for hiding this comment

Uh oh!

eduard13 Mar 21, 2020

Choose a reason for hiding this comment

Uh oh!

eduard13 Mar 21, 2020

Choose a reason for hiding this comment

Uh oh!

eduard13 Mar 21, 2020

Choose a reason for hiding this comment

Uh oh!

eduard13 Mar 21, 2020

Choose a reason for hiding this comment

Uh oh!

eduard13 Mar 21, 2020

Choose a reason for hiding this comment

Uh oh!

eduard13 Mar 21, 2020

Choose a reason for hiding this comment

Uh oh!

vasilii-b commented Mar 22, 2020

Uh oh!

eduard13 commented Mar 23, 2020

Uh oh!

magento-engcom-team commented Mar 23, 2020

Uh oh!

magento-engcom-team commented Mar 23, 2020

Uh oh!

eduard13 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

engcom-Echo commented Mar 24, 2020

Uh oh!

engcom-Echo commented Mar 25, 2020

Uh oh!

engcom-Echo commented Mar 26, 2020

Uh oh!

slavvka left a comment

Choose a reason for hiding this comment

Uh oh!

eduard13 left a comment

Choose a reason for hiding this comment

Uh oh!

magento-engcom-team commented Mar 28, 2020

Uh oh!

slavvka commented Mar 28, 2020

Uh oh!

magento-engcom-team commented Mar 29, 2020

Uh oh!

m2-assistant bot commented Mar 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

eduard13 left a comment •

edited

Loading