Skip to content
This repository has been archived by the owner on Sep 7, 2023. It is now read-only.

[fix] oscar theme, remove inline style attributes (CSP compliants) #1954

Merged
merged 11 commits into from Jun 30, 2020
Merged

[fix] oscar theme, remove inline style attributes (CSP compliants) #1954

merged 11 commits into from Jun 30, 2020

Conversation

return42
Copy link
Contributor

@return42 return42 commented May 15, 2020

Inline styles are blocked by default with Content Security Policy (CSP). Move the rest of inline styles to CSS and correct the HTML template of the oscar preference page. Fix of the error messages from Chrome based browsers::

.. Refused to apply inline style because ..

see comment: #1949 (comment)

Merged into the searx-next branch / preview: https://darmarit.org/searx/preferences

return42 and others added 7 commits May 15, 2020 09:57
Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
Inline styles are blocked by default with Content Security Policy (CSP).  Move
the rest of inline styles to CSS and correct the HTML template of the oscar
preference page.

Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
the CSS files has been build by:

    $ make themes.oscar

Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
@return42
Copy link
Contributor Author

return42 commented Jun 10, 2020

@asciimoo can you please merge this PR first before adding any more patches to the oscar theme. I am tired to wake up every morning and to have solve conflicts in the min.js files first / thanks!

@return42
Copy link
Contributor Author

return42 commented Jun 10, 2020

BTW 2c6531b breaks the pep8 test of all the PRs awaiting in the pipe. See 2c6531b#diff-65ed3496c99aad64ac6a6e71504e0101R40

OT / it is not only this PR, and I am really sorry to say:

I'm getting more and more frustrated. I invested months of work to improve searx substantially (see searx-next merged PRs). But my PRs are ignored, which entails considerable maintenance costs: Instead of reviewing PRs, changes are shot in, which additionally increase the organizational overhead. Something is going wrong if we are three developers here and generate an organizational effort that is otherwise only known from sluggish, large companies.

@HLFH
Copy link
Collaborator

HLFH commented Jun 12, 2020

Hi @asciimoo,

I'm getting more and more frustrated. I invested months of work to improve searx substantially (see searx-next merged PRs). But my PRs are ignored, which entails considerable maintenance costs: Instead of reviewing PRs, changes are shot in, which additionally increase the organizational overhead. Something is going wrong if we are three developers here and generate an organizational effort that is otherwise only known from sluggish, large companies.

Can this be fixed @asciimoo?

Thanks!

@return42
Copy link
Contributor Author

WTH I am saying!?! .. commit 4ca0d8c is next one, directly shot in without a review process.

I feel like i am trapped in the groundhog-day time loop .. next manual merge cc721b5 of a conflict caused by 4ca0d8c.

@HLFH thanks for your support .. but I guess all we are writing here goes directly into @asciimoo 's spam folder .. also tried personal mails, seems also going into spam .. sometimes (really rare) I get a answer (with little informative value, not really a knowledge exchange) .. mostly I am ignored.

@return42
Copy link
Contributor Author

OK, another conflict and this PR is ignored .. I gave up.

@@ -1 +1 @@
.searx-navbar{background:#29314d;height:2.3rem;font-size:1.3rem;line-height:1.3rem;padding:.5rem;font-weight:700;margin-bottom:.8rem}.searx-navbar a,.searx-navbar a:hover{margin-right:2rem;color:#fff;text-decoration:none}.searx-navbar .instance a{color:#01d7d4;margin-left:2rem}#main-logo{margin-top:20vh;margin-bottom:25px}#main-logo>img{max-width:350px;width:80%}*{border-radius:0!important}html{position:relative;min-height:100%;color:#29314d}body{font-family:Roboto,Helvetica,Arial,sans-serif;margin-bottom:80px;background-color:#fff}body a{color:#08c}.footer{position:absolute;bottom:0;width:100%;height:60px;text-align:center;color:#999}input[type=checkbox]:checked+.label_hide_if_checked,input[type=checkbox]:checked+.label_hide_if_not_checked+.label_hide_if_checked{display:none}input[type=checkbox]:not(:checked)+.label_hide_if_not_checked,input[type=checkbox]:not(:checked)+.label_hide_if_checked+.label_hide_if_not_checked{display:none}.onoff-checkbox{width:15%}.onoffswitch{position:relative;width:110px;-webkit-user-select:none;-moz-user-select:none;-ms-user-select:none}.onoffswitch-checkbox{display:none}.onoffswitch-label{display:block;overflow:hidden;cursor:pointer;border:2px solid #FFF!important;border-radius:50px!important}.onoffswitch-inner{display:block;transition:margin .3s ease-in 0s}.onoffswitch-inner:before,.onoffswitch-inner:after{display:block;float:left;width:50%;height:30px;padding:0;line-height:40px;font-size:20px;box-sizing:border-box;content:"";background-color:#EEE}.onoffswitch-switch{display:block;width:37px;background-color:#01d7d4;position:absolute;top:0;bottom:0;right:0;border:2px solid #FFF!important;border-radius:50px!important;transition:all .3s ease-in 0s}.onoffswitch-checkbox:checked+.onoffswitch-label .onoffswitch-inner{margin-right:0}.onoffswitch-checkbox:checked+.onoffswitch-label .onoffswitch-switch{right:71px;background-color:#A1A1A1}.result_header{margin-top:0;margin-bottom:2px;font-size:16px}.result_header .favicon{margin-bottom:-3px}.result_header a{color:#29314d;text-decoration:none}.result_header a:hover{color:#08c}.result_header a:visited{color:#684898}.result_header a .highlight{background-color:#f6f9fa}.result-content,.result-format,.result-source{margin-top:2px;margin-bottom:0;word-wrap:break-word;color:#666;font-size:13px}.result-content .highlight,.result-format .highlight,.result-source .highlight{font-weight:700}.result-source{font-size:10px;float:left}.result-format{font-size:10px;float:right}.external-link{color:#069025;font-size:12px;margin-bottom:15px}.external-link a{margin-right:3px}.result-default,.result-code,.result-torrent,.result-videos,.result-map{clear:both;padding:.5em 4px}.result-default:hover,.result-code:hover,.result-torrent:hover,.result-videos:hover,.result-map:hover{background-color:#f6f9fa}.result-images{float:left!important;width:24%;margin:.5%}.result-images a{display:block;width:100%;background-size:cover}.img-thumbnail{margin:5px;max-height:128px;min-height:128px}.result-videos{clear:both}.result-videos hr{margin:5px 0 15px 0}.result-videos .collapse{width:100%}.result-videos .in{margin-bottom:8px}.result-torrent{clear:both}.result-torrent b{margin-right:5px;margin-left:5px}.result-torrent .seeders{color:#2ecc71}.result-torrent .leechers{color:#f35e77}.result-map{clear:both}.result-code{clear:both}.result-code .code-fork,.result-code .code-fork a{color:#666}.suggestion_item{margin:2px 5px;max-width:100%}.suggestion_item .btn{max-width:100%;white-space:normal;word-wrap:break-word;text-align:left}.result_download{margin-right:5px}#pagination{margin-top:30px;padding-bottom:60px}.label-default{color:#a4a4a4;background:0 0}.result .text-muted small{word-wrap:break-word}.modal-wrapper{box-shadow:0 5px 15px rgba(0,0,0,.5)}.modal-wrapper{background-clip:padding-box;background-color:#fff;border:1px solid rgba(0,0,0,.2);border-radius:6px;box-shadow:0 3px 9px rgba(0,0,0,.5);outline:0 none;position:relative}@media screen and (max-width:75em){.img-thumbnail{object-fit:cover}}.infobox .panel-heading{background-color:#f6f9fa}.infobox .panel-heading .panel-title{font-weight:700}.infobox p{font-family:"DejaVu Serif",Georgia,Cambria,"Times New Roman",Times,serif!important;font-style:italic}.infobox .btn{background-color:#2ecc71;border:none}.infobox .btn a{color:#fff;margin:5px}.infobox .infobox_part{margin-bottom:20px;word-wrap:break-word;table-layout:fixed}.infobox .infobox_part:last-child{margin-bottom:0}.search_categories,#categories{text-transform:capitalize;margin-bottom:.5rem;display:flex;flex-wrap:wrap;flex-flow:row wrap;align-content:stretch}.search_categories label,#categories label,.search_categories .input-group-addon,#categories .input-group-addon{flex-grow:1;flex-basis:auto;font-size:1.2rem;font-weight:400;background-color:#fff;border:#ddd 1px solid;border-right:none;color:#666;padding-bottom:.4rem;padding-top:.4rem;text-align:center;min-width:50px}.search_categories label:last-child,#categories label:last-child,.search_categories .input-group-addon:last-child,#categories .input-group-addon:last-child{border-right:#ddd 1px solid}.search_categories input[type=checkbox]:checked+label,#categories input[type=checkbox]:checked+label{color:#29314d;font-weight:700;border-bottom:#01d7d4 5px solid}#main-logo{margin-top:10vh;margin-bottom:25px}#main-logo>img{max-width:350px;width:80%}#q{box-shadow:none;border-right:none;border-color:#a4a4a4}#search_form .input-group-btn .btn{border-color:#a4a4a4}#search_form .input-group-btn .btn:hover{background-color:#2ecc71;color:#fff}.custom-select{appearance:none;-webkit-appearance:none;-moz-appearance:none;font-size:1.2rem;font-weight:400;background-color:#fff;border:#ddd 1px solid;color:#666;background:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAA8AAAAPCAQAAACR313BAAAABGdBTUEAALGPC/xhBQAAACBjSFJNAAB6JgAAgIQAAPoAAACA6AAAdTAAAOpgAAA6mAAAF3CculE8AAAAAmJLR0QA/4ePzL8AAAAJcEhZcwAABFkAAARZAVnbJUkAAAAHdElNRQfgBxgLDwB20OFsAAAAbElEQVQY073OsQ3CMAAEwJMYwJGnsAehpoXJItltBkmcdZBYgIIiQoLglnz3ui+eP+bk5uneteTMZJa6OJuIqvYzSJoqwqBq8gdmTTW86/dghxAUq4xsVYT9laBYXCw93Aajh7GPEF23t4fkBYevGFTANkPRAAAAJXRFWHRkYXRlOmNyZWF0ZQAyMDE2LTA3LTI0VDExOjU1OjU4KzAyOjAwRFqFOQAAACV0RVh0ZGF0ZTptb2RpZnkAMjAxNi0wNy0yNFQxMToxNTowMCswMjowMP7RDgQAAAAZdEVYdFNvZnR3YXJlAHd3dy5pbmtzY2FwZS5vcmeb7jwaAAAAAElFTkSuQmCC) 96% no-repeat}.search-margin{margin-bottom:.6em}#advanced-search-container{display:none;text-align:left;margin-bottom:1rem;clear:both}#advanced-search-container label,#advanced-search-container .input-group-addon{font-size:1.2rem;font-weight:400;background-color:#fff;border:#ddd 1px solid;border-right:none;color:#666;padding-bottom:.4rem;padding-right:.7rem;padding-left:.7rem}#advanced-search-container label:last-child,#advanced-search-container .input-group-addon:last-child{border-right:#ddd 1px solid}#advanced-search-container input[type=radio]{display:none}#advanced-search-container input[type=radio]:checked+label{color:#29314d;font-weight:700;border-bottom:#01d7d4 5px solid}#check-advanced{display:none}#check-advanced:checked~#advanced-search-container{display:block}.advanced{padding:0;margin-top:.3rem;text-align:right}.advanced label,.advanced select{cursor:pointer}.cursor-text{cursor:text!important}.cursor-pointer{cursor:pointer!important}pre,code{font-family:'Ubuntu Mono','Courier New','Lucida Console',monospace!important}.lineno{margin-right:5px}.highlight .hll{background-color:#ffc}.highlight{background:#f8f8f8}.highlight .c{color:#556366;font-style:italic}.highlight .err{border:1px solid #ffa92f}.highlight .k{color:#BE74D5;font-weight:700}.highlight .o{color:#d19a66}.highlight .cm{color:#556366;font-style:italic}.highlight .cp{color:#bc7a00}.highlight .c1{color:#556366;font-style:italic}.highlight .cs{color:#556366;font-style:italic}.highlight .gd{color:#a00000}.highlight .ge{font-style:italic}.highlight .gr{color:red}.highlight .gh{color:navy;font-weight:700}.highlight .gi{color:#00a000}.highlight .go{color:#888}.highlight .gp{color:navy;font-weight:700}.highlight .gs{font-weight:700}.highlight .gu{color:purple;font-weight:700}.highlight .gt{color:#04d}.highlight .kc{color:#BE74D5;font-weight:700}.highlight .kd{color:#BE74D5;font-weight:700}.highlight .kn{color:#BE74D5;font-weight:700}.highlight .kp{color:#be74d5}.highlight .kr{color:#BE74D5;font-weight:700}.highlight .kt{color:#d46c72}.highlight .m{color:#d19a66}.highlight .s{color:#86c372}.highlight .na{color:#7d9029}.highlight .nb{color:#be74d5}.highlight .nc{color:#61AFEF;font-weight:700}.highlight .no{color:#d19a66}.highlight .nd{color:#a2f}.highlight .ni{color:#999;font-weight:700}.highlight .ne{color:#D2413A;font-weight:700}.highlight .nf{color:#61afef}.highlight .nl{color:#a0a000}.highlight .nn{color:#61AFEF;font-weight:700}.highlight .nt{color:#BE74D5;font-weight:700}.highlight .nv{color:#dfc06f}.highlight .ow{color:#A2F;font-weight:700}.highlight .w{color:#d7dae0}.highlight .mf{color:#d19a66}.highlight .mh{color:#d19a66}.highlight .mi{color:#d19a66}.highlight .mo{color:#d19a66}.highlight .sb{color:#86c372}.highlight .sc{color:#86c372}.highlight .sd{color:#86C372;font-style:italic}.highlight .s2{color:#86c372}.highlight .se{color:#B62;font-weight:700}.highlight .sh{color:#86c372}.highlight .si{color:#B68;font-weight:700}.highlight .sx{color:#be74d5}.highlight .sr{color:#b68}.highlight .s1{color:#86c372}.highlight .ss{color:#dfc06f}.highlight .bp{color:#be74d5}.highlight .vc{color:#dfc06f}.highlight .vg{color:#dfc06f}.highlight .vi{color:#dfc06f}.highlight .il{color:#d19a66}.highlight .lineno{-webkit-touch-callout:none;-webkit-user-select:none;-khtml-user-select:none;-moz-user-select:none;-ms-user-select:none;user-select:none;cursor:default;color:#556366}.highlight .lineno::selection{background:0 0}.highlight .lineno::-moz-selection{background:0 0}.highlight pre{background-color:#282C34;color:#D7DAE0;border:none;margin-bottom:25px;font-size:15px;padding:20px 10px}.highlight{font-weight:700}.table>tbody>tr>td,.table>tbody>tr>th{vertical-align:middle!important}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we wouldn't commit the minified files this PR wouldn't conflict as often but it also could have been merged a while ago.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes minified and binaries are causing problems in collaboration .. as far as I know @asciimoo do not want to have a build process / sorry, can't remember where I have read this. May be he as changed his opinion about .. @asciimoo ?

Ah, forgot, we will not get an answer ..

@asciimoo
Copy link
Member

@asciimoo can you please merge this PR first before adding any more patches to the oscar theme. I am tired to wake up every morning and to have solve conflicts in the min.js files first / thanks!

My main priority was to resolve issues required to release 1.0.0 , sorry if it caused problems to you. Probably the conflicts are mostly in the minified resources what can be easily resolved.

I'm getting more and more frustrated.

Im so sad reading this. Apologies for this. If it's my fault, please help me with suggesting solutions to avoid such situations.

I invested months of work to improve searx substantially (see searx-next merged PRs). But my PRs are ignored

They are not ignored. I'm trying to do everything in my limited time and sometimes it's really hard to prioritize and allocate enough time to digest multiple thousand lines long PRs and 30-40-50 mails daily basis just about searx while I also have other projects/jobs.

also tried personal mails, seems also going into spam

I think It isn't my fault that gmail classified your emails as spam. Probably It's a mail server misconfiguration issue.

@asciimoo do not want to have a build process

I don't think I've ever said something like this, but whether it happened or not, now I'd like to explicitly state that I'm not against any kind of improvement that can help the development or the job of the contributors/maintainers.
I know that there are so many unresolved issues around the development, let's focus on finding solutions for these.

@return42
Copy link
Contributor Author

@asciimoo thanks for taking this into account. I will answer the more general aspects in #2025

My main priority was to resolve issues required to release 1.0.0

Can we please add CSP to that list. I have seen that others add exceptions for the style to the HTTP headers, but I think it is better to isolate style into CSS files.

I think It isn't my fault that gmail classified your emails as spam. Probably It's a mail server misconfiguration issue.

Its first time I hear about such problems with my mail server and I do not know how to trainee g-mails's spam filter. I know from many filters, that they wont filter out mails from senders which are in the contatcs. May it helps if you add my e-mail address to your contacts.

I don't think I've ever said something like this ..I know that there are so many unresolved issues around the development, let's focus on finding solutions for these.

Ah, OK .. that is fine for me, lets add a build process to eliminate conflicts in build products. I started issue #2034 .. if you have some ideas, your remarks are welcome / or better say: your remarks are needed :)

@return42 return42 reopened this Jun 29, 2020
@dalf
Copy link
Contributor

dalf commented Jun 29, 2020

Except the conflicting files, LGTM.

@asciimoo
Copy link
Member

<style> tags in the HTML don't compromise the CSP?

@dalf
Copy link
Contributor

dalf commented Jun 29, 2020

<style> tags in the HTML don't compromise the CSP?

@asciimoo , are you referencing this pattern

    <noscript>
        <style type="text/css">
...
        </style>
    </noscript>

?

True, no javascript doesn't mean the browser doesn't support CSP (typically with Firefox with the uMatrix extension).

For now, this PR fixes the more common use case: a browser with Javascript and CSP support.

@asciimoo
Copy link
Member

@asciimoo , are you referencing this pattern

Yup

For now, this PR fixes the more common use case: a browser with Javascript and CSP support.

Okay, I have no problem with merging the current state, but keep in mind these <style> tags too.

@SuperSandro2000
Copy link

You can find a very good documentation here. The paragraph Unsafe inline styles is probably the most intersting.

@return42
Copy link
Contributor Author

I just wanted to remark, that I think; setting style-src 'unsafe-inline' is not really the best solution.

@SuperSandro2000
Copy link

I did not want to suggest that. It is just Mozillas doc on the CSP style-src and what falls under 'unsafe-inline'.

@asciimoo
Copy link
Member

@return42 could you please rebase it for the last time to be able to merge it?

@return42
Copy link
Contributor Author

could you please rebase it for the last time to be able to merge it?

@asciimoo yes, done / see f14a7ad

@asciimoo
Copy link
Member

Great, thanks!
Could you please rebase in the future instead of merging back the master into the feature branch? It keeps the git tree clean. Currently our master branch looks like a busy train station:
x

@asciimoo asciimoo merged commit 6163bd6 into searx:master Jun 30, 2020
@SuperSandro2000
Copy link

The image looks like art. 😂

@return42
Copy link
Contributor Author

Could you please rebase in the future instead of merging back the master into the feature branch?

I see, sorry .. ATM this is a result of the way I merge PRs with searx-next branch.

I do not have a clue how I could rebase a public branch (PR) which is already merged into another public branch (searx-next)

@SuperSandro2000, @dalf and me discussed this subject beginning near by #1985 (comment)

Does anyone have a clue, how I can organize my searx-next branch and the branches from PRs without getting in conflict with the golden rule; “No one shall rebase a shared branch”? I guess I'm not the only one asking this question.

@asciimoo
Copy link
Member

What is the purpose of your searx-next branch? Cherry-picking is also an option which can work here I think.

@return42
Copy link
Contributor Author

What is the purpose of your searx-next branch?

When I had a few PRs in the loop, I started that branch. It purpose is to merge PRs aimed to merge at asciimoo/master (in general, not restricted to PRs of mine).

This branch has it's reference instance at https://darmarit.org/searx/ and the documentation of the brand is available at https://return42.github.io/searx/

Cherry-picking is also an option which can work here I think.

This is was others also suggested, but sooner or later it will end in a mess, so I prefer to track branches (pull requests).

@asciimoo
Copy link
Member

s branch has it's reference instance at https://darmarit.org/searx/

I see, thanks for the clarification.

but sooner or later it will end in a mess

Maintaining two different forks always has its challenges, but the current solution also causes mess in our side. Please try cherry-picking and lets continue the brainstorming if it's too uncomfortable.

@return42
Copy link
Contributor Author

but the current solution also causes mess in our side. ... Please try cherry-picking

I will stop adding PRs to searx-next .. as long as I do not have a more comfortable method than cherry-pickng / I have to think about it .. remarks & hints are welcome ..

@asciimoo
Copy link
Member

What is exactly the problem with cherry-pick? I think it's quite convenient to create a clear feature branch, pick some commits and create/update the PR, you can even rebase that branch without conlficting with your searx-next branch.

@return42
Copy link
Contributor Author

return42 commented Jul 1, 2020

@asciimoo thanks for taking up the brainstorm with me .. I have a doubt that a understand your suggestions. Some word to clarify ..

On the one side; I understand that you like to see a branch cleared up before merge into master. In my self-next branch there are two PRs open which not have been merged into asciimoo/master (#1985 and #1938). I can clean up theses branches if I got the OK for a merge. Since there are not many clones from searx--next exists, it should also OK to rebase searx-next in context of this both PRs .. I think this problem is solved.

On the other side, I want to progress with brainstorming about maintaining a branch like searx-next.

The motivation of the searx-next branch is to have a branch with a reference installation and this branch should also a testing area for PRs (other branches) before I (we) merge them into asciimoo/master . For smaller PRs without any special in, we do not need such a reference installation but there are also examples where it is good to have one, e.g. when I try to fix CSP errors or just test some issue users added to our issue tracker (often, they use an unmaintained instance and the reported bug have already been fixed). BTW: is there a chance to have searx.me back?

What is exactly the problem with cherry-pick?

It does not track .. if you have a bunch of PRs (branches) which are patched in the review phase, you have to track all branches manual.

I think it's quite convenient to create a clear feature branch, pick some commits and create/update the PR, you can even rebase that branch without conlficting with your searx-next branch.

I haven't understand in deep .. what I know (may I'm wrong) .. when you track a branch, and this branch is rebased or the history is changed in any manner, you lost the anchor where the external branch was started initial and than you do not have a chance to merge (pull) from that branch .. I'm wrong .. or did I misunderstood your suggestion?

@return42 return42 deleted the csp-oscar-theme branch July 15, 2020 16:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants