Geelong
Account created on 28 February 2008, almost 16 years ago
#

Merge Requests

Recent comments

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Looks like this is outdated, but thanks for all your other contributions!

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Nigel Cunningham β†’ made their first commit to this issue’s fork.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

I've just done this for the 3.x branch as it stands, will add credit for both of you. Thanks for raising the issue.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

I've sought to reproduce this using CKEditor 5 / Drupal 10 and not needed the patch. Could someone please provide more detailed information regarding the circumstances in which it is needed?

Thanks!

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

I believe this has been implemented but the ticket never got updated. Closing as fixed.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Closing as outdated; the code has moved on too much and PHPCS is passing on all files at the moment.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

D9 is now unsupported. Let's close this as Won't fix.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Done in the 3.x branch. I didn't use the patch from hree, sorry (things had moved on too much).

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

The PR got worked on and merged. I think this can be closed now.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

I agree.

I'm planning on implementing a plugin / event based way of altering the content. The link extractor is currently a prime target and modifications like this would fit in well to such a rework.

Hopefully not far away, though I'm checking through the issue queue for other ideas before starting on this.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Rebased to 3.x, updated for changes I've made today and merged. Thanks Chris!

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Nigel Cunningham β†’ made their first commit to this issue’s fork.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

This is now implemented and in 3.x dev.

Please test.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Awesome. Thanks for the reply!

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Apologies for the long delay in answering.

The theme hooks printable uses work just look other theme hooks so I can only suggest that you follow the normal methods for debugging why your hook isn't being used (eg https://www.drupal.org/docs/develop/theming-drupal/twig-in-drupal/debugg... β†’ ).

We did have a buggy suggestion hook (which I've just fixed) that tried to put printable_ as a prefix for links; it should be on the end if anywhere (since it uses a view mode).

I hope that's helpful.

I'll close this as "Works as designed" but please feel free to comment further if you need help.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Thanks for this report.

I've just pushed a series of commits that fix this issue. As part of the fix, they add a new option to the link extractor config form that lets you choose whether to exclude the Printable links from the list. It defaults to enabled for backwards compatibility.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Would you consider doing a release with just this fix? Until it is released, people upgrading to 10.2 will need to also apply the merged patch instead of just updating the module to the latest stable.

Thanks!

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Made a new 2.0 release supporting D9 and 10. Thanks for the prod!

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Oh, still using CKEditor 4? Yeah, ok.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Static version of MR for use in composer patching.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Attaching static version of MR for composer patch.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

I know it's too late now but trailing commas are ugly, unnecessary extra work for the parser and only useful when you're adding another arg later on. They should be removed from the array requirement, not added elsewhere!

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Fix released as 2.4.2 after consulting on the #security-team Slack channel about whether it needed to go through the Security release process (11.53am AEDT, 16 December 2023)

For future reference, if security issues occur in our module or upstream, can we discuss them privately first to be sure about whether we need to through that process, please? I can be reached via my D.O contact form.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Merging, thanks very much.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

@jacksick, do you have plans to do a release any time soon, please?

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Nigel Cunningham β†’ made their first commit to this issue’s fork.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

I've just committed a patch that adds a Google font to the default CSS (which has better international support). Please try 3.x-dev.

Thanks!

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Hmm, not sure what's going on there. :)

Commited; thanks!

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Committed, along with some fixes to get everything rendering consistently with the other link extractors.

Re the removal of printable URLs from the list, please either submit a patch to add that option for all of the link extractors or override it in your own code.

Thanks for the patch!

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

> I understand the desire for consistency (I share it!), but if all the project would consist of is a wrapper around a composer requirement, that seems heavy-handed to me.

I don't think any of them would just be a composer requirement. Each has a plugin definition and I think the non-puppeteer ones could also perhaps do with a config form to handle the different possibilities for accessing images (chroot jail etc) that still seem to cause people issues with getting images into their PDFs.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

It is a separate module because when I first created it, I wasn't even sure that what I was seeking to do would work (Puppetteer integration was just part of something much bigger). Now that it's all complete and in production, yes - we could just make it another part of the pdf_api module.

That said, I don't think everyone is going to want puppeteer support automatically installed any more than I want/need domPDF, mPDF and wkhtmltoPDF for the Puppeteer project, and Puppeteer support doesn't just imply the Rialto and Puphpeteer modules in the vendor directory. It also needs node, Puppeteer and Chrome themselves, which should really be installed outside of the docroot.

At this stage it sounds wiser and perhaps safer (security issue in an upstream library, anyone?) to me to give the end user the choice regarding what libraries they install.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

I think whatever we do, we should do it consistently - I believe most users tend to use mPDF or domPDF or ...

Perhaps the best idea would be to spin all of the libraries off to separate projects? They could be a pdf_api_* namespace?

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Ah, so many tools, so many complaints :)

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Thanks for the reply.

Ok, perhaps I made a mess somewhere there :) You can't force push to DO so I don't think that is what would have happened, but so long as we end up with usable code...

Thanks again!

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Hi Joseph.

I'm trying to reproduce this now, without success so far, using your sample form. Any further hints you might be able to provide would be appreciated. I'm using the dev version of config entity revisions.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Thanks for the patch but the fix was already added in f060e4cf.

Regards,

Nigel

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Thanks for the patch, but that's already committed in the 2.0.x branch. I'm doing some more work now and hope to do a release today.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Morning Grimreaper.

Apologies first for the delay in getting back to this.

I've just enabled Obfuscate afresh in my module dev install, added the Obfuscate filter to the end of the Basic HTML configuration and put "write us" in the source of a normal node's WYSIWYG, using basic HTML for the format. On the resulting page, the source shows:


write us

Could I get you to point out what I'm doing differently? The commit I'm on is fa512f3b (2.0.x branch)

Thanks!

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Here's an updated version of #67 that addresses the BlockContent issue I mentioned in the previous comment.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

The patch also needs more work. It removes BlockContent::invalidateBlockPluginCache without modifying the invocations of that method.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Would it be feasible to make the change optional? Sometimes, such as when testing in the context of a local development environment, you might validly want to just enter user@localhost.

Regards,

Nigel

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Yeah, I want to do a bunch of cleanups to make the whole module nicer and cleaner. Finding the time though... :)

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Sorry for the glacially slow response.

Did you get it working for you? There's a link to a tutorial on the project home page that should get you going. The other thing you might try is adjusting the Full Viewport option or the Blur option to "Scrolling (Full viewport)"

Since this issue is really old, I'm going to assume you've moved on from here and close it, but if that's not the case please feel free to reopen and we can continue the discussion.

Regards,

Nigel

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

I've applied a bunch of fixes today; would you please retest? I'll mark the issue as fixed in the meantime.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Actually, I'm pretty sure this is fixed now so I'll close the issue. If you're of a different opinion, please reopen it and provide info to help me reproduce what you see.

Thanks!

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Hi there.

I'm applied a bunch of fixes to the module today and believe you should no longer see the issue. I'll close this as fixed for now but please reopen if that's not your experience too.

Regards,

Nigel

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

I've made other changes in the area of caching and am seeing changes to background image settings take effect without requiring a cache clear. I'll mark this as fixed but please reopen if you retest and find any issues still exist.

Thanks!

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Commited; thanks!

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Apologies for the slow reply.

This should be fixed in the dev branch now; I'm planning on doing a new release too shortly.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Apologies for the slow reply.

This should be fixed in the dev branch now; I'm planning on doing a new release too shortly.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Thanks for the report.

I saw this too today and have committed the same change; it is working for me now on the dev branch. I hope to make a new release shortly. If you have any further issues, please let me know.

Thanks again!

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Thanks for raising the issue but I don't believe this will be an issue with the module itself; it doesn't try to mess with permissions so they'll depend on your particular filesystem's configuration.

I'll close the issue as "works as designed" but if you have info that proves me wrong, please feel free to reopen it.

Thanks!

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

The comment that the Interface should be used is totally right. I applied a fix for this in dev separately to this patch but will still give credit here.

Thanks!

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Thanks for your report.

Line 437 doesn't match current dev so I assume you're actually trying the 2.0.1 release.

If that's not correct, could I get you to provide steps for reproducing the issue from a fresh install? I haven't succeeded in reproducing it so far.

I'm planning on continuing to look at other issues and hope to provide a new release today too; assuming that happens, please try that instead.

Thanks again!

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Hi again.

I was wrong in the previous comment - we did have some code to do what you ask, but it was broken, perhaps by changes upstream since it was written. Whatever the case, I've now pushed fixes/improvements to the dev branch.

If you use the full control (the multiple fields version), it will now populate the other fields when you type an address into the field field and pick an entry from the autocomplete.

Could you check out whether it achieves what you're after?

Thanks!

Nij

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Thanks for your query.

The address only version does a simplified version - a single line field that lets you enter just a full address on one line (eg 234 Somewhere Street, Someville 3000, Victoria). The full version provides separate fields that let you enter a house number separately to the street name and so on. The full version does also include the "Full version" field but that's only because it includes all fields that the API provides.

Regarding whether you can access the individual parts of the address post-validation, we don't do anything there at the moment but it looks to be possible. The AddressFinder API provides a way to get location metadata (https://addressfinder.com.au/api/au/location/metadata/). I'll see what I can do for you.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Patch looks good to me and has been successfully tested by all the above; marking RTBC.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

This will need to be custom code in your particular website, sorry - the javascript that's needed, CSS selectors and other details will depend upon the user's website (on another website, someone might want one type of accordion expanded and another not), so it shouldn't be part of the module.

You might also be able to achieve this just using CSS in a print media query.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Oh, I'm sorry - I forgot all about trying your form.

I'll seek to do so and hopefully the work around won't be needed.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

There's already support for logging in without a token via the TfaLoginInterface's loginAllowed method. This should be factored in.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

I recently updated the dev branch, modifying the defaults to use Google fonts that include the required characters.

Please give it a try.

Regards,

Nigel

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Came to the realisation that's it's caused by an outdated version of the patch for 3181631.

Closing.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Hi @Grimreaper.

I've just tried applying the patch and am not seeing a case where it still makes any difference - it applies, but doesn't seem to make any difference to the output. Could you help me out with a case where you're still seeing an issue without the patch being applied?

Thanks!

Nigel

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Thanks for the additional information.

Could you also describe your configuration a bit more:

Are you running PHP in a chroot jail or such like?
How is Printable configured?
Assuming you're generating a PDF, what version of the pdf_api module are you using and which library?

Thanks!

Nigel

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

I've added a commit checking the text isn't empty before invoking the page_text function but I don't think it will help in the initially reported case - could you provide more detail regarding what you have set as your page header. (I'm seeking to know what the text variable in that function contains).

Thanks!

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Hi again.

I've taken a look today at adding this, and would like to do some tidyups before merging it - will it be a problem to you if I commit a modified version?

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Nigel Cunningham β†’ made their first commit to this issue’s fork.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

This is fixed with the release of 2.4.0, thanks.

πŸ‡¦πŸ‡ΊAustralia Nigel Cunningham Geelong

Closing as outdated since PHPCS issue have been fixed elsewhere. Thanks for your work though - will see if I can credit you elsewhere.

Production build https://api.contrib.social 0.61.6-2-g546bc20