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.

Sign up for GitHub

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

Jump to bottom

Update background.js #5

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Update background.js #5

wants to merge 5 commits into from

Conversation

cesarmtnez
Copy link

Apply styles to all tab frames

@cesarmtnez
Update background.js
d2a562d 
Apply styles to all tab frames
@fej-snikduj
Copy link
Owner

Apply styles to all tab frames

Hi @cesarmtnez - can you give me an example to test this out on? I'm not entirely sure how to reproduce - is this referring to iframes?

@cesarmtnez
Copy link
Author

Hello @fej-snikduj.

I usually have to use a internal page with the old tag <frameset>. Let me show you an example without content:

<html xmlns="http://www.w3.org/1999/xhtml" style="cursor: inherit;">
<head>
    <title>Foo</title>
    <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
</head>
<frameset rows="63,*" frameborder="NO" border="0" framespacing="0" style="opacity: 1; pointer-events: inherit;">
    <frame src="notFound.html" name="topFrame">
    <frame src="notFound.html" name="mainFrame">
</frameset>
<noframes>
    <body>
    Unsuported frames
    </body>
</noframes>
</html>

In those kind of pages the extension does not show any style.
With the param allFrames:true (PR) the extension prints the style for each frame.

@cesarmtnez cesarmtnez mentioned this pull request Jun 25, 2020
Open
@cesarmtnez
Copy link
Author

Hello @fej-snikduj

Have you been able to test my example?

Regards.

@fej-snikduj
Copy link
Owner

fej-snikduj commented Jul 16, 2020 via email

@fej-snikduj
Copy link
Owner

@cesarmtnez - I tested this out. It works as expected - though I'm concerned it my not be the expected behavior for all the users out there - some users may be surprised that their frames are now displaying a border. I think we should make this an option that is defaulted false so all current behavior remains the same for current users - what are your thoughts?

@cesarmtnez
Update popup.js
e703f6c 
Allow to configure 'all frames' border
@cesarmtnez
Update popup.html
8c7b31f 
Allow to configure 'all frames' border
@cesarmtnez
Update background.js
4f2bba9 
Allow to configure 'all frames' border
@cesarmtnez
Update manifest.json
901a66a 
Allow to configure 'all frames' border
@cesarmtnez
Copy link
Author

Sorry @fej-snikduj , but I was on vacation.
Yes, you are right and I think your proposed solution is a good idea.
I have implemented it and you can see it in this current pull request.

@cesarmtnez
Copy link
Author

Hi @fej-snikduj , have you been able to review the pull request?

Greetings

@bender270
Copy link

Hi, any news about this PR approval?
It would be useful for some users, for me included

Regards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cesarmtnez @fej-snikduj @bender270