Addthis cuts off query from url

As investigated today, I recommend to switch to always add this url.
With typical Drupal development patterns, query strings can be considered part of the URL, change the results (such as a view filter) and should not be automatically dropped, ever. This fix makes this explicit.

However, please add a comment about the original issue any why we enforce it.. and link to this issue.

Finally, sure, this kind of things should be easily alterable. But this should be covered in a followup. (please create)

Actually the special option to add the meta info into the inline attributes then has a different meaning:
It now is only to suppress the global OG tag overwriting.
I think, we should have here some better solution, at least detects clashes and warn. Possibly have some hierarchy / weights. Possibly also invert the checkbox to "promote to page OG tags". (followup)

As discussed:

- Drop the current flag
- Ad the view mode to $context before calling the alter hook
- support the following view modes:

full: Default, display og tags if first like now, always add atributes
only_og: like now, only og tags, no visible output
no_attributes: like full, but don't add attributes
no_og: like full, but don't add og tags.

Then document those somewhere, README.txt probably.

Committed with the attached documentation changes. No need to be specific about entity reference there, view modes can always be used.

Yes, definitely. That is what powers smid=1, this change only made it work out of the box, thanks to the attributes that are how there by default.

For now I have ported only the test. I'm not sure what to do about the view_mode array and where is/should be defined the sharemessage_entity_info() method. Any suggestion?

Let's see the tests running.

+++ b/src/Tests/ShareMessageViewModesTest.php
@@ -0,0 +1,104 @@
+  protected function assertShareButtons($sharemessage, $addthis_attributes = FALSE, $invert = FALSE) {

This function contains too much logic. It reads "assertShareButtons()" and adding an $invert is really strange to read code without knowing the implementation.

Make it more self explaining. Some code duplication in tests does not hurt. Or create wrapper methods with nice names.

Split assertShareButtons(): getRawHtmlIconeStyle(), assertShareButton()

Hm, i think i didn't explain well.

I mean we should do less arguments and make the method more self explanatory by name:
- assertShareButton()
- assertNoShareButton()
- assertOGTags()
- assertNoOGTags()
Internally you might want to have assertShareButtonHelper() or assertOGTagsHelper() with an extra argument to do the inverse check to avoid code duplication.
But the test code only calls the nicely named methods with only few arguments.

Created the 6 assert methods mentioned in #19 and fixed the test method.

Yeah now it reads much nicer!

Now just the functionality is missing..? ;-)

And about assertOGTags():

1. +++ b/src/Tests/ShareMessageViewModesTest.php
   @@ -0,0 +1,170 @@
   +  protected function assertOGTagsHelper($title, $not_exists = TRUE) {
   

   Hm, we might want to verify more elements from OG than just the title. Or you think it is enough because covered elsewhere in more detail?

2. +++ b/src/Tests/ShareMessageViewModesTest.php
   @@ -0,0 +1,170 @@
   +    $meta_title = '<meta property="og:title" content="' . $title . '" />';
   

   This loooks much risky if $title contains special characters such as "'" or '"'. The example should cover it to proof we have no XSS problems. And $title should be encoded here.

Tried to port the rest.

Oh, ok I'll do it next week :)

Interdiff:

• modified assertOGTags() / assertNoOGTags() / assertOGTagsHelper() parameter ($property, $value)

As discussed with @miro_dietiker we might want to move the OGTags assertion methods to a base class, since there is also other tests regarding these OG properties. Created issue here: #2638624: Move assertOGTags() from test to base class

Also we have added a @todo in Drupal\sharemessage\Tests\ShareMessageSettingsTest::testShareMessageOGTags() to add a coverage for a safer check for the special characters in OG Tags.
Created issue here: #2638628: Add test coverage for special characters in OG tags

The test is still failing and I'm still not sure if the last ported parts are ok. I thought that it should be added some lines in AddthisSettingsForm and sharemessage.addthis.yml to set which view_mode to use or something like that, am I right?

Looks fine to me. Now let's make it pass... ;-)

Fixed!

Created an issue to move the ShareButtons assertion methods to the base class in #2639208: Move assertShareButtons() from test to base class, already uploaded the patch. Need to be approved and committed so I can reroll the patch of this issue and make this test to pass.

So referred things progressed... Now back to work in this issue and make it pass? ;-)

Yep, let's do it! :D

Rerolled, fixed the additional AddThis attributes in the base class and the tests.

Fixed! :D

Fixed absolute url bug.

I like! Committed! :-)