Access element through jQuery

[sikker]

The contents of element here is a regular DOM node rather than a jQuery object, and as such the .prop method isn't available, causing "element.prop() is not a function" errors in the browser. The solution is to just access it through jQuery.

[adragus-inviqa]

Appreciate the effort @sikker, but this PR does the same thing as #4022 and #4102.

[sikker]

I see, no reason to have another one then.

Although one could wonder why it's taking a month to accept a single line
code change.

On Fri, May 13, 2016 at 4:59 PM, adragus-inviqa [email protected]
wrote:

Appreciate the effort @sikker https://github.com/sikker, but this PR
does the same thing as #4022
#4022 and #4102
#4102.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#4496 (comment)

[adragus-inviqa]

Yep, we're all wondering that.
Although not all one-liners are changes with no consequences, this one seems pretty obvious.
Then again, isn't this what they all say?

PS: Close this PR if you're ok with the other.

[vkorotun]

I see two pull requests are competing here )
#4022 do the same thing.
The first one with all checks green will be merged within appropriate time sloth.

Issue with this one: "Contributor License Agreement is not signed yet."

[sikker]

As has already been mentioned in this very thread, there are more than two
competing.

The trouble is that two months for a single line code change to be approved
is ridiculous for a project the size of Magento 2.

On Thu, May 26, 2016 at 3:41 PM, Vitalii Korotun [email protected]
wrote:

I see two pull requests are competing here )
#4022 #4022 do same thing.
The first one with all checks green will be merged within appropriate time
sloth.

Issue with this one: "Contributor License Agreement is not signed yet."

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#4496 (comment)

[poojacignex]

I need this change in my Project.
So when will I get patch to resolve this issue as its not recommend to change in core module.

Thanks in Advance

[jvreeken]

Just as a suggestion... if there are iframes on the page that are not of the same origin... CORS kicks in and there will be an error saying it couldn't access the frame to cache it basically...

to fix this, I would recommend fixing it by testing whether the iframe is of the same origin first before running the rest of the function:

if ($.nodeName(element, "iframe") && $(element).prop('src').indexOf(window.location.hostname) != -1) { return; }

this should be added right at the beginning of the check:
(function lookup(element) { if ($.nodeName(element, "iframe") && $(element).prop('src').indexOf(window.location.hostname) != -1) { return; } $(element).contents().each(function (index, el) {

[jvreeken]

page-cache.js.zip

[PascalBrouwers]

@sikker tnx for the fix. I added it to my 2.1.0 installation.

[OZZlE]

@jvreeken why not make a PR for this? so I can append .patch to it and fetch it :D I'm not used to making PRs to magento's structure yet.. don't know where I can find the module repo..

[PascalBrouwers]

@OZZlE if you want a PR, try looking at this one: https://github.com/magento/magento2/pull/4022/files

[OZZlE]

@PascalBrouwers it's not the same thing.. I added a PR myself now: #7914

[franckgarnier21]

When does this PR will be release in Magento 2.X version ? Still not here on 2.1.7

[ishakhsuvarov]

Hi @sikker
We have just noticed that Contributor License Agreement was not signed for this Pull Request.

Could you please follow the CLA Assistant link and sign it?

Thank you for collaboration.