-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[KleinanzeigenBridge] random improvements and fixes #4821
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Pull request artifacts
last change: Sunday 2025-11-16 16:00:59 |
Previous MR RSS-Bridge#4820 introduced a bug where the URI wasn't getting expanded. This is because it is obtained from a non-standard data-uri attribute which defaultLinkTo() doesn't support. On top of that: - sanitizes the HTML in Content - use a longer Description found in JSON - fix timestamp processing, including for relative Today and Yesterday strings - move media to enclousures - be explicit about elements chosen to augument the description - simplify the image URL processing
01717c8 to
a6bd9f4
Compare
Mynacol
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR still has CI failures, please look at them and my comments. Thanks!
|
|
||
| if ($element->find('img', 0)) { | ||
| //enhance img quality. Cannot use convertLazyLoading() here due to non-standard URI suffix in srcset. | ||
| $item['enclosures'] = [preg_replace('/rule=\$_\d+\.AUTO/i', 'rule=$_57.AUTO', $element->find('img', 0)->getAttribute('src')) . '#.image']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the . '#.image' part doing at the end?
| ); //enhance img quality | ||
|
|
||
| $item['content'] = '<img src="' . $imgUrl . '"/>' . $element->find('div.aditem-main', 0)->outertext; | ||
| $item['uri'] = urljoin($this->getURI(), $element->getAttribute('data-href')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could fail if Kleinanzeigen chooses to make the value an absolute URL? Add a small if or ternary operator thin to catch that.
|
|
||
| $item['timestamp'] = strtotime($dateString); | ||
| } else { | ||
| $item['timestamp'] = time(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be better to not include a timestamp if not available/on failure? Otherwise, the timestamp of existing ads could change over time? Not sure here.
| ) | ||
| ); //enhance img quality | ||
|
|
||
| $item['content'] = '<img src="' . $imgUrl . '"/>' . $element->find('div.aditem-main', 0)->outertext; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would still add the image into the content HTML in addition to the enclosure. Depending on RSS reader, the display of this stuff can vary widely.
If you want, you can add a bridge option to control that.
Previous MR #4820 introduced a bug where the URI wasn't getting expanded. This is because it is obtained from a non-standard data-uri attribute which defaultLinkTo() doesn't support.
On top of that: