Skip to content
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

added optional options to noprocess #2954

Merged
merged 4 commits into from
Jul 7, 2020
Merged

added optional options to noprocess #2954

merged 4 commits into from
Jul 7, 2020

Conversation

ricardo118
Copy link
Contributor

This enables the option &noprocess to be expanded to &noprocess=id,hl

Example:

[PlayStore](https://play.google.com/store/apps/details?id=org.jitsi.meet&hl=de&target=_blank&noprocess)

Current result link: https://play.google.com/store/apps/details?id=org.jitsi.meet&hl=de&target=_blank - not opening in new tab

This will work as intended by not processing the id, however the target also gets no processed.

Using

[PlayStore](https://play.google.com/store/apps/details?id=org.jitsi.meet&hl=de&target=_blank&noprocess=id)

Will skip the id processing, and still process the target. Resulting in a better end result link.

New result link: https://play.google.com/store/apps/details?id=org.jitsi.meet&hl=de - opening in new tab successfully

This is in reference to the following issues:

#2839 - FIXED

#2882

https://discordapp.com/channels/501836936584101899/501854266873348112/729728029063905345

@ricardo118 ricardo118 requested a review from rhukster July 6, 2020 16:49
@ricardo118
Copy link
Contributor Author

theres some bugs found in discord currently testing

@ricardo118
Copy link
Contributor Author

ricardo118 commented Jul 6, 2020

[noprocess](https://play.google.com/store/apps/details?id=org.jitsi.meet&hl=de&target=_blank&noprocess)  
result:  `https://play.google.com/store/apps/details?id=org.jitsi.meet&hl=de&target=_blank`  
should have `target=_blank` in url and **not open in a new tab**  

[noprocess=id](https://play.google.com/store/apps/details?id=org.jitsi.meet&hl=de&target=_blank&noprocess=id)  
result:  `https://play.google.com/store/apps/details?id=org.jitsi.meet&hl=de`  
should have `id=org.jitsi.meet&hl=de` in url and **open in a new tab**  
  
[only target](https://play.google.com/store/apps/details?target=_blank)  
result:  `https://play.google.com/store/apps/details`  
should  **open in a new tab**  
  
[Browser](https://meet.weikamp.biz/Support?rel=nofollow&target=_blank)  
result:  `https://meet.weikamp.biz/Support`  
should  **open in a new tab**  

Test cases above all working as intended

@rhukster
Copy link
Member

rhukster commented Jul 6, 2020

be great if you could add these to the test suite @ricardo118

@rhukster
Copy link
Member

rhukster commented Jul 6, 2020

Also will need some updates to learn to show how it can be used with options

Signed-off-by: Andy Miller <rhuk@mac.com>
Signed-off-by: Andy Miller <rhuk@mac.com>
@rhukster
Copy link
Member

rhukster commented Jul 7, 2020

I added some tests, and used those to confirm the functionality is good. perhaps you can use these for future reference.

@rhukster rhukster merged commit b94c4e7 into 1.7 Jul 7, 2020
@ghost
Copy link

ghost commented Apr 26, 2021

I needed to remove this "automatic extraction of links attributes" so I removed all of them in the configuration.

Un fortunately, as there is no attributes, the configuration element is no more an array but "null" and I got Exception "in_array() expects parameter 2 to be array, null given" (/system/src/Grav/Common/Page/Markdown/Excerpts.php at line 131) :

            // Loop through actions for the image and call them.
            foreach ($actions as $attrib => $value) {
                if (!in_array($attrib, $skip)) {
                    $key = $attrib;
 
                    if (in_array($attrib, $valid_attributes, true)) { // <---- this one is the 131
                        // support both class and classes.
                        if ($attrib === 'classes') {
                            $attrib = 'class';
                        }
                        $excerpt['element']['attributes'][$attrib] = str_replace(',', ' ', $value);
                        unset($actions[$key]);
                    }
                }
            }

My workaround without changing the code is to add a random string as a valid attribute in the hope no website will use it as valid attribute (but as I choose it randomly, chances are really tiny).

To fix it in the code, I changed the 117th line as follows (adding the ?: [] at the end) :

            $valid_attributes = $grav['config']->get('system.pages.markdown.valid_link_attributes') ?: [] ;

(I was going to submit a pull request with this change but did not find the Excerps.php file in develop or 1.7 branches so I don't know how to fix it in the github code).

@mahagr
Copy link
Member

mahagr commented Apr 26, 2021

@ricardo118 Can you take a look?

@ricardo118
Copy link
Contributor Author

ya will do

@ricardo118
Copy link
Contributor Author

I needed to remove this "automatic extraction of links attributes" so I removed all of them in the configuration.

Un fortunately, as there is no attributes, the configuration element is no more an array but "null" and I got Exception "in_array() expects parameter 2 to be array, null given" (/system/src/Grav/Common/Page/Markdown/Excerpts.php at line 131) :

            // Loop through actions for the image and call them.
            foreach ($actions as $attrib => $value) {
                if (!in_array($attrib, $skip)) {
                    $key = $attrib;
 
                    if (in_array($attrib, $valid_attributes, true)) { // <---- this one is the 131
                        // support both class and classes.
                        if ($attrib === 'classes') {
                            $attrib = 'class';
                        }
                        $excerpt['element']['attributes'][$attrib] = str_replace(',', ' ', $value);
                        unset($actions[$key]);
                    }
                }
            }

My workaround without changing the code is to add a random string as a valid attribute in the hope no website will use it as valid attribute (but as I choose it randomly, chances are really tiny).

To fix it in the code, I changed the 117th line as follows (adding the ?: [] at the end) :

            $valid_attributes = $grav['config']->get('system.pages.markdown.valid_link_attributes') ?: [] ;

(I was going to submit a pull request with this change but did not find the Excerps.php file in develop or 1.7 branches so I don't know how to fix it in the github code).

i just tested this, and while yes what you say does happen, i fixed the error without having any issues by simply setting the config to
valid_link_attributes: [] instead of valid_link_attributes: null.

However, admin saves it as null, so i believe the check suggested here is valid. Pushed a change here 2813934

@rhukster rhukster deleted the ricardo-patch-noprocess branch October 23, 2024 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants