SeoMaestro

[Wanze]

5 hours ago, Mikie said:

Hi @Wanze, getting an error with 0.8.0 update with the structured data breadcrumb template using TemplateLatteReplace module. Its fine as I can ignore that file with Latte using the module settings, but maybe changes relying on rendering templates like this should default to unchecked, as I assume same issue would occur with other template systems.

SeoMaestro (including myself) knows nothing about TemplateLatteReplace, so I cannot influence settings of this module. I am not sure why the LatteReplace would try to render the structured data template, I assume it's because SeoMaestro internally uses a ProcessWire TemplateFile to render the markup: https://github.com/wanze/SeoMaestro/blob/master/src/StructuredData/BreadcrumbStructuredData.php#L52-L55

There is nothing I can do from Seo Maestro's side about this, and I assume you will have problems with other modules as well that are using ProcessWire's TemplateFile for rendering. My TemplateEngineFactory module providing template engines like Twig, Smarty, Pug etc. does not suffer from this problem, because it hooks after Pager::render rather than TemplateFile::render.

Cheers

[Mikie]

18 hours ago, Wanze said:

SeoMaestro (including myself) knows nothing about TemplateLatteReplace, so I cannot influence settings of this module. I am not sure why the LatteReplace would try to render the structured data template, I assume it's because SeoMaestro internally uses a ProcessWire TemplateFile to render the markup: https://github.com/wanze/SeoMaestro/blob/master/src/StructuredData/BreadcrumbStructuredData.php#L52-L55

There is nothing I can do from Seo Maestro's side about this, and I assume you will have problems with other modules as well that are using ProcessWire's TemplateFile for rendering. My TemplateEngineFactory module providing template engines like Twig, Smarty, Pug etc. does not suffer from this problem, because it hooks after Pager::render rather than TemplateFile::render.

Hi @Wanze, I was just suggesting for new functionality maybe it is best to default to off initially, eg for this update have Breadcrumb unchecked by default and allow users to opt in and test. Nothing to do with controlling other modules settings. My rationale for this is that since this module directly renders output, updates that add functionality are essentially breaking in that they affect rendered html. I would think only essential updates that are needed for the module to function would default on. Just a thought.

You are right re TemplateLatteReplace, This line:
https://github.com/rolandtoth/TemplateLatteReplace/blob/cbac28b93e74e3ba51f7ce32f374c385e406d09a/TemplateLatteReplace.module#L186
checks against current page template not the template called by the TemplateFile class. This is fixable by getting the basename from $templateFile->filename instead, will submit an issue to @tpr.

Thanks for the response!

[teppo]

Hey @Wanze! These aren't really issues, more general feedback, so I thought I'd post them here. I've been evaluating this module as a solution for handling metadata, and as such have finally had some time to actually dig into it's numerous features – and while I see a lot of interesting stuff here, there are a few things that could perhaps use some polishing, in my opinion ?

So one thing that confused me initially was the use of word "inherit". I managed to miss the point about default values being set template level in README (which obviously was a major reason for my confusion... but to my defence: when evaluating a new module I like to do it without reading too much first, since that's how most regular users – as in clients – are going to be using it anyway), so I imagined that the values would be inherited from parent pages. (Seemed a bit weird obviously, but whatever.)

Now that I've double-checked the docs and understand that those values are inherited from the template, I'm actually wondering if you might consider calling this something else – perhaps "default"? It would also be good to explain somehow (in context) what "inherit" means, and where the value is inherited from?

Note: I get that anyone installing the module and having access to template settings should probably realise what this is all about, but I for one don't give clients access to template settings (I see that as the developers' territory). Thus "inherit" doesn't make much sense to them – and since it's not explained in the admin (and they're unlikely to figure out that the site is using SeoMaestro and dig out the modules README or this thread), it can indeed be quite confusing.

--

Another source of confusion for me was the Sitemap feature, mainly because it didn't seem to do anything. Now, looking into the source code, I see that you're using file_put_contents() – so apparently the module expects write access to the site root? It might be a good idea to add a check to see if this really is the case. At least I assume that this was the problem – didn't see any errors, but in my case Apache or PHP never have write access to directories with executable code, so if the module did try to write in those directories, it must've failed.

(Personally I like the "hook 404 page and serve fake files" approach more when it comes to things like this – it doesn't require write access, and tends to work better overall in different environments.)

--

Just some initial observations – hope you don't mind poking around. You're doing great job with this module! ?

[Wanze]

Hi @teppo

Thanks for your valuable feedback!

On 7/2/2019 at 10:21 PM, teppo said:

Now that I've double-checked the docs and understand that those values are inherited from the template, I'm actually wondering if you might consider calling this something else – perhaps "default"? It would also be good to explain somehow (in context) what "inherit" means, and where the value is inherited from?

What do you think about calling the label "Inherit default value"? For the explanation, we could use an InputfieldMarkup prior to any meta data fields (similar to the one when editing the default values on field-level).

It has been mentioned before that it should be possible for content editors to edit default values without having permission to edit fields or templates. I think the module will (should) include another Process module, which could offer this configuration GUI, along with other interesting things such as SEO reports ?

On 7/2/2019 at 10:21 PM, teppo said:

Another source of confusion for me was the Sitemap feature, mainly because it didn't seem to do anything. Now, looking into the source code, I see that you're using file_put_contents() – so apparently the module expects write access to the site root? It might be a good idea to add a check to see if this really is the case. At least I assume that this was the problem – didn't see any errors, but in my case Apache or PHP never have write access to directories with executable code, so if the module did try to write in those directories, it must've failed.

(Personally I like the "hook 404 page and serve fake files" approach more when it comes to things like this – it doesn't require write access, and tends to work better overall in different environments.)

The sitemap does not have to be in the site root necessarily, you could also enter a path to a writeable folder. But you're right, I actually never thought of this problem and also do not check it ? At least this should be mentioned in a description or note. I am aware of the 404 hook, but I do not like it very much, it feels "wrong". Other hooks listening to the 404's might get triggered as well, but then it's no longer a 404 after returning the sitemap. Let me think about this ? 

Cheers

[teppo]

23 hours ago, Wanze said:

What do you think about calling the label "Inherit default value"? For the explanation, we could use an InputfieldMarkup prior to any meta data fields (similar to the one when editing the default values on field-level).

It has been mentioned before that it should be possible for content editors to edit default values without having permission to edit fields or templates. I think the module will (should) include another Process module, which could offer this configuration GUI, along with other interesting things such as SEO reports ?

All of these sound like great ideas ?

23 hours ago, Wanze said:

The sitemap does not have to be in the site root necessarily, you could also enter a path to a writeable folder. But you're right, I actually never thought of this problem and also do not check it ? At least this should be mentioned in a description or note. I am aware of the 404 hook, but I do not like it very much, it feels "wrong". Other hooks listening to the 404's might get triggered as well, but then it's no longer a 404 after returning the sitemap. Let me think about this ? 

Sure, no worries. I get that there are ups and downs to both approaches.

My use case may be a bit unorthodox, but again I never give ProcessWire or the Apache user write access to anywhere that isn't strictly necessary (so only the assets directory, or parts of it). In my opinion giving PHP write access to executable code is asking for trouble. Additionally ProcessWire writing files to the disk can be problematic if you're using automated deployment process – depending on your setup those files may get lost in every deployment, or at least require some extra logic to work properly.

Just some points to consider, and of course folks like me can always leave the sitemap feature to another module or custom code. In my opinion the minimum level would be checking if the file can be added, and then somehow warning the user if it can't. Just for reference, here's how I ended up implementing a similar permission check ?

[elabx]

HI! Awesome module @Wanze!

Does anyone know if this module allows a fallback field  (or lists of them) within the page if for example, the description is not set?

[Wanze]

Thanks @elabx

The module allows to fallback to fields using placeholders. For example, if your page has a field lead_text, you can set the default value of the meta description to {lead_text}. If the content editor omits the meta description of a page, it will fallback to the lead text. It is not possible to fallback to multiple fields though. Does this answer your question?

Cheers

[Sanyaissues]

Edit: Forget it. The error disappear.

Hi Does anybody understand why this error? SeoMaestro 0.8.0 / PW 3.0.135

Fatal error: Method SeoMaestro\PageFieldValue::__toString() must not throw an exception, caught ErrorException: chdir(): open_basedir restriction in effect. File(/) is not within the allowed path(s): (/var/www/vhosts/xxxxxx.com/:/tmp/) in /var/www/vhosts/grupopages.com/sites/xxxxxxx.com/wire/core/WireDebugInfo.php on line 0

[teppo]

1 hour ago, Sanyaissues said:

Edit: Forget it. The error disappear.

Hi Does anybody understand why this error? SeoMaestro 0.8.0 / PW 3.0.135

Glad to hear that the error has disappeared ?

The problem might've had something to do with your environment, but the resulting error is an interesting one. In this case something attempted to change current working directory into one that wasn't allowed, so PHP threw an exception – and since throwing exceptions from __toString() is not allowed, that resulted in fatal error instead. This seems like an unexpected result, so it might be worth investigating if it can be avoided, though I'm not sure if there's any sensible way around this. As long as __toString() does anything even remotely complicated, there's a risk of running into this issue.

[sz-ligatur]

Hi Maestros,

I just recognized, that the meta_title_format field ist not correctly interpreted in language variants – instead the default language is used in the outputted markup. Can anybody confirm this? Any ideas where and how to fix this behavior? Please see attached screenshot. (SeoMaestro 0.8.0, pwire 3.0.123) 

[Wanze]

Hi @sz-ligatur

Looks like you've found a bug! The relevant code is here: https://github.com/wanze/SeoMaestro/blob/master/src/MetaSeoData.php#L30-L35
It looks like these lines do not grab the field's value in the current language.

Can you try to change the code like this:

// Old
if ($field->get('meta_title_format')) {
  $value = str_replace('{meta_title}', $value, $field->get('meta_title_format'));
}

// New
$metaTitleFormat = $field->get('meta_title_format' . $this->getCurrentLanguageId()) ?: $field->get('meta_title_format');
if ($metaTitleFormat) {
  $value = str_replace('{meta_title}', $value, $metaTitleFormat);
}

Not tested and written in the browser, but maybe it works ?

Cheers

[sz-ligatur]

… and it works – thank you so much – your fix arrived just in time :o)

[Wanze]

8 hours ago, sz-ligatur said:

… and it works – thank you so much – your fix arrived just in time ?

Glad it works, thanks for the feedback! This fix will be integrated in the next release, so you'll be able to update safely.

Cheers

[antpre]

Hi,

thanks for the module that look spromising.
I have a error after installing the module on a fresh install of PW 3.0.1232 (latest master) with php 7.0.3

Parse Error: syntax error, unexpected '?' (line 50 of /var/www/vhosts/XXXXXXXX/httpdocs/site/modules/SeoMaestro/InputfieldSeoMaestro.module.php)

And there is a warning message : SeoMaestro - Requires module "PHP>=7.0.0" before it can be installed

Which I am not sure what it means.

Any idea what's happening.

Thanks in advance for your help.

Edited August 15, 2019 by antpre

[Wanze]

Hi @antpre

Your PHP version is most likely below 7.0, try to update. Some more information about this error can be found here:

Cheers

[antpre]

Thanks a lot Wanze,

I actually thaught I was running this site on php 7  but it was not !!!

Thanks again have a great day

[wbmnfktr]

Heads up: SeoMaestro and ProcessWire 3.0.139 (DEV) don't like each other.

Just ran into this issue - luckily in a local testing environment - while creating/editing a SeoMaestro field.

Fatal Error: Uncaught Error: Call to a member function prepend() on null in /home/alexander/www/ssw.test/wire/core/Fields.php:1066
Stack trace:
#0 /home/alexander/www/ssw.test/wire/modules/Process/ProcessField/ProcessField.module(1412): ProcessWire\Fields->getCompatibleFieldtypes(Object(ProcessWire\Field))
#1 /home/alexander/www/ssw.test/wire/core/Wire.php(380): ProcessWire\ProcessField->___buildEditFormBasics()
#2 /home/alexander/www/ssw.test/wire/core/WireHooks.php(813): ProcessWire\Wire->_callMethod('___buildEditFor...', Array)
#3 /home/alexander/www/ssw.test/wire/core/Wire.php(442): ProcessWire\WireHooks->runHooks(Object(ProcessWire\ProcessField), 'buildEditFormBa...', Array)
#4 /home/alexander/www/ssw.test/wire/modules/Process/ProcessField/ProcessField.module(1020): ProcessWire\Wire->__call('buildEditFormBa...', Array)
#5 /home/alexander/www/ssw.test/wire/core/Wire.php(380): ProcessWire\ProcessField->___buildEditForm()
#6 /home/alexander/www/ssw.test/wire/core/WireHooks.php(813): ProcessWire\Wire->_callMethod('___bui (Zeile 1066 in /home/alexander/www/ssw.test/wire/core/Fields.php)

Diese Fehlermeldung wurde angezeigt wegen: Sie sind als Superuser angemeldet. Fehler wurde protokolliert.

Update: 3.0.140 same behaviour

[Tyssen]

On 9/5/2019 at 5:07 AM, wbmnfktr said:

Heads up: SeoMaestro and ProcessWire 3.0.139 (DEV) don't like each other.

Update: 3.0.140 same behaviour

Just encountered the same.

[dynweb]

Just replace in FieldtypeSeoMaestro.module.php, line 119:

        // return null;
        return $this->wire(new Fieldtypes());

This works for me...

[wbmnfktr]

Thanks @dynweb! 

Looks good so far. Will give it a try the next days and see if there are any issues with that fix.

[adrian]

On 9/4/2019 at 12:07 PM, wbmnfktr said:

Heads up: SeoMaestro and ProcessWire 3.0.139 (DEV) don't like each other.

I think this is probably just a side-effect of this recently introduced core bug: https://github.com/processwire/processwire-issues/issues/979

[Mike-it]

Same for me, after Update to: 3.0.140

Does anybody encounter also analogue problem with mapmarker?

I've just tryed to open the field and:

Fatal Error: Uncaught Error: Call to a member function prepend() on null in /var/www/www____/wire/core/Fields.php:1066
Stack trace:
#0 /var/www/mywww/wire/modules/Process/ProcessField/ProcessField.module(1412): ProcessWire\Fields->getCompatibleFieldtypes(Object(ProcessWire\Field))
#1 /var/www/mywww/wire/core/Wire.php(380): ProcessWire\ProcessField->___buildEditFormBasics()
#2 /var/www/mywww/wire/core/WireHooks.php(813): ProcessWire\Wire->_callMethod('___buildEditFor...', Array)
#3 /var/www/mywwwit/wire/core/Wire.php(442): ProcessWire\WireHooks->runHooks(Object(ProcessWire\ProcessField), 'buildEditFormBa...', Array)
#4 /var/www/mywww/wire/modules/Process/ProcessField/ProcessField.module(1020): ProcessWire\Wire->__call('buildEditFormBa...', Array)
#5 /var/www/mywww/wire/core/Wire.php(380): ProcessWire\ProcessField->___buildEditForm()
#6 /var/www/mywww/wire/core/WireHooks.php(813): ProcessWire\Wire->_callMethod('___buildEditFor...', Array)
#7 /var/www/mywww/ (line 1066 of /var/www/www.440-hz.it/wire/core/Fields.php)

[Tyssen]

15 hours ago, dynweb said:

Just replace in FieldtypeSeoMaestro.module.php, line 119:

        // return null;
        return $this->wire(new Fieldtypes());

This works for me...

That solves the editing of the field problem, but if I visit a page which has the field on it, I get:

Quote

Call to undefined method stdClass::getArray()
107:            $formManager->populateValues($form, $pageValue->getArray());

 

[wbmnfktr]

Can't confirm that issue here. Looks pretty normal to me.

[Wanze]

Hi guys,

I just released version 0.9.0 of the module which includes some fixes and new additions, you can find all the details in the changelog.

What's new?

The module offers a new section "Webmaster Tools" when editing a field, allowing you to set verification codes for Google and Bing. If set, these codes are rendered as additional meta tags. I had to refactor the UI in order to separate the webmaster tools from editing the default values. My solution was to separate them via fieldsets:

Cheers