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

fixed Firefox popup size issue #89

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

Conversation

denadai2
Copy link
Contributor

Fixes #40

@denadai2
Copy link
Contributor Author

To explain better this issue: the popup was displaying the web version, with huge problems of usability.
This is a temporarily fix in order to make the extension working in Firefox browsers in Linux systems.

(Why is the popup spaced 7px from the border?)

@oderwat
Copy link

oderwat commented Nov 25, 2014

Is that fixing Windows 7 FF 33 too? I just saw that this is still not working. I was just concerned about OSX which got a fix some weeks ago.

@denadai2
Copy link
Contributor Author

I think yes, but I can't try it :((

@oderwat
Copy link

oderwat commented Nov 25, 2014

I think I still have a buildable copy (where I could apply your patch) on my development system and a win7 vm. I may try it later tonight.

@denadai2
Copy link
Contributor Author

nice

@@ -83,7 +83,7 @@ var updateLoginState = function () {
}
helper.tabs.create({url: helper.getURL('html/change-password.html' + hash)});
} else {
if (($(window).width() > POPUP_WIDTH) && debugMode === false) { // Check to see if this in browser window, not popup
if (($(window).width() > POPUP_WIDTH+7) && debugMode === false) { // Check to see if this in browser window, not popup
Copy link
Collaborator

Choose a reason for hiding this comment

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

please declare this as a constant at the top. Magic numbers in the middle of code like this is scary, especially when they're repeated. Also spaces around the +.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, its also possible this means the POPUP_WIDTH should be adjusted directly? Does it get used elsewhere?

We had another bug report (issue #96) that someone thinks this should fix. If we can clean up this pull request I'm happy to merge it and attempt a release.

@denadai2
Copy link
Contributor Author

Fixes #96

@cvladan
Copy link

cvladan commented Jan 9, 2015

Will someone merge this? Please!
This pull request is "sleeping" for more than 2 months,
and it looks like it solved a HUGE BUG on Firefox.

Is anybody using Firefox at all, anymore?

Please, faster

@denadai2
Copy link
Contributor Author

denadai2 commented Jan 9, 2015

@kafene
Copy link

kafene commented Jan 18, 2015

I couldn't get the extension to build on my machine, so I edited the code right in my firefox profile folder since the PR was only 4 characters, and it works for me.

@teh
Copy link

teh commented Feb 4, 2015

ping.

Would it make sense to fork this and take over maintenance?

@denadai2
Copy link
Contributor Author

denadai2 commented Feb 4, 2015

then we would not have any official release :)

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.

Firefox 31-33 in Linux mint: font/login problem
7 participants