Closed Bug 819871 Opened 12 years ago Closed 12 years ago

Nightly: can not login at Mail.com

Categories

(Core :: Layout, defect)

Product:
Core
Component:
Layout
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla21
Tracking Flags:
Tracking Status
firefox18 --- unaffected
firefox19 + verified
firefox20 + verified
firefox21 --- verified

People

(Reporter: mye, Assigned: mounir)

References

Details

(Keywords: regression)

Attachments

(2 files)

testcase
(deleted), text/html
Details
Patch
(deleted), patch
dbaron
: review+
bzbarsky
: review+
akeybl
: approval-mozilla-aurora+
akeybl
: approval-mozilla-beta+
Details | Diff | Splinter Review 
User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:20.0) Gecko/20121209 Firefox/20.0
Build ID: 20121209030817

Steps to reproduce:

Try to login at Mail.com


Actual results:

I get a blank white page, nothing happens, I'm not forwarded to my inbox

issue persists since Wednesday December 5th


Expected results:

I should get forwarded to my inbox

May be the same issue as https://bugzilla.mozilla.org/show_bug.cgi?id=751806
I still can't login at mail.com

this seems to be a regression from https://bugzilla.mozilla.org/show_bug.cgi?id=751806

please have a look
Confirmed.

STR:
1) Open http://www.mail.com/int/
2) Login with guest account:
Username: bugzilla2@mail.com
Password: Tyf/X&pUq1

Result: after a few sec, Mail.com fails to redirect to the inbox and displays a blank page.

Regression range:
m-c
good=2012-11-09
bad=2012-11-10
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=90cea19e27e2&tochange=a47525b93528
Status: UNCONFIRMED → NEW
tracking-firefox19: --- → ?
tracking-firefox20: --- → ?
Ever confirmed: true
Keywords: regression, regressionwindow-wanted
Error console:

Timestamp: 15/12/2012 00:54:03
Error: SyntaxError: An invalid or illegal string was specified
Source File: http://s.uicdn.com/service.mail.com/client/static/script/compiled-gecko.c0b0e58c053e-4009950451.js
Line: 4636

"{"+c+"}",a.cssRules.length):a.addRule(b,c)},removeRule:function(a,b){if(qx.core.Environment.get("html.stylesheet.deleterule")){var c=
Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/eef85b630817
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/19.0 Firefox/19.0 ID:20121109022331
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/1fdb059f5ae6
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/19.0 Firefox/19.0 ID:20121109023532
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=eef85b630817&tochange=1fdb059f5ae6
Blocks: 737786
Keywords: testcase-wanted
Attached file testcase (deleted) — 
When insert an old rule(i.e. "input:-moz-placeholder") in a style sheet, it becomes the error.
Hrm.  So that's a bit unfortunate...  I wonder whether we can just make insertRule not throw on invalid syntax or something in general.  :(

The other option is to keep parsing the old pseudo-class but just never match it...  Ugly, I know.
I think it would be better to get the site to fix the bug.
OK, let's start there.

Frank, since you use the site, are you willing to contact them to complain about this bug in their code?  They're depending on non-standard functionality that was removed...
Assignee: nobody → english-us
Component: Untriaged → English US
Product: Firefox → Tech Evangelism
Version: 20 Branch → unspecified
hey Boris,

sure I would. But I guess if a representative of Mozilla contacts Mail.com and explains the issue, that Firefox users from the near future on will no longer be able to use their services, it would have a bigger effect, than an anonymous user complaining about something code-related.

Just tell me if you could do this, otherwise I'll try to.

thanks for looking into the issue
Frank
Frank, I'm planning to contact them too.  Some sites listen more to us, some listen more to their own users, so we should do both.
Great. That will do.

Would you be so kind and give me the email adress, which you will be writing to? So we make sure, that both of our mails get to the same representative and are not spread across the company..

And it would be nice to know what you are writing (with regards to technical details), to put some of that into my own mail to make it seem that I know what I'm talking about ;)
Just use their contact form here: http://service.mail.com/shareFeedback.html?edition=int&lang=en&device=desktop

And put in your message a link to this bug report with a brief summary.
Not tracking, evang issue.  If the site is relying on non-standard functionality that has been removed I suspect we are not considering putting that functionality back to be an option so for now please keep trying to reach out to the site. If this is still an issue when 20 gets to Aurora we can revist other options weighing the number of users we suspect will be affected against the risks of perhaps backing out whatever removed the functionality.
tracking-firefox19: ? → -
tracking-firefox20: ? → -
How are we going to remember to revisit it?  I thought the point of tracking was precisely to track things like that...
Hey guys,

after getting back their automatic reply, I send them the inquiry again. I'll report as soon as I get an answer.
I just sent this through the feedback form (after making up a mail.com address so I could submit it):

Dear mail.com,

It looks like there is a script on your login page that attempts to insertRule a rule with the non-standard ":-moz-placeholder" pseudo-class in it into a stylesheet.  Unfortunately, support for that non-standard pseudo-class has been removed in Firefox 19 (which will ship in mid-February 2013) and newer.  This makes the insertRule call throw an exception, which is not caught by the script, and that makes it impossible to log in to the site.

See https://bugzilla.mozilla.org/show_bug.cgi?id=819871 for more information, or feel free to contact me directly.
well it seems that Mail.com is not interested in resolving the issue, since I haven't heard from them since my initial request.

Since I need to access Mail.com a dozen times a day and have used Nightly ever since and want to continue using Nightly and not Beta or Release versions, it seems I have to leave Firefox for another browser.

I totally understand that Mozilla can't put ancient code into the source of Firefox just to please a handful of users. So either I'm start using Palemoon or something similar (while hoping that the defective code segments won't get ported to this builds), or make a cut and switch to Opera although I don't really like that browser.
PaleMoon is going to have the same behavior here...
But note that I've heard nothing from them either.

re-nominating for tracking.  This problem has been on Aurora for a while now (it's a problem in 19), and is about to appear on Beta, so I'm not sure why comment 20 is talking about 20 going to Aurora....
tracking-firefox19: - → ?
tracking-firefox20: - → ?
(In reply to Boris Zbarsky (:bz) from comment #19)
> But note that I've heard nothing from them either.

Do we have any low risk options in code?
(bug 823444 is similarly affected, so it's possible that this will pop up a lot more post-release)
> Do we have any low risk options in code?

The lowest risk option I can think of is re-adding the pseudo-class and just having it match nothing (see comment 6)...
(In reply to Alex Keybl [:akeybl] from comment #21)
> (bug 823444 is similarly affected, so it's possible that this will pop up a
> lot more post-release)

well, just in case you don't know: Mail.com is owned by GMX...
OS: Windows 8 → All
Hardware: x86_64 → All
According to comment 6, there are two ways to fix this in our code:
- change insertRule behaviour;
- parse ":-moz-placeholder" pseudo-class (but do nothing).

Another solution is to hope that those websites will fix their code as soon as a big chunk of their users will complain but we risk to also lose those users...

Can we get a fix ready and wait until we are farther in the Beta process to decide whether we should land this or not?

As a side note, I wonder if we shouldn't rename the pseudo-element ::placeholder to prevent that problem to happen again... :-/
Assignee: english-us → nobody
Component: English US → Layout
Product: Tech Evangelism → Core
(In reply to Mounir Lamouri (:mounir) from comment #24)
> Can we get a fix ready and wait until we are farther in the Beta process to
> decide whether we should land this or not?

We don't like to hold off landing a fix based upon something out of our control. Please move forward with the lowest risk fix here.
Assignee: nobody → mounir
We need this fix ready to land in the next week, to make it into Beta 3. We shouldn't be landing this change later than that in the cycle, given the possibility of regression.
Boris and David, given that a prefix bites us again, I would think we should do something with the ::-moz-placeholder that has been introduced instead of :-moz-placeholder. Whether we rename it to ::placeholder or we disable it for the release.

I believe we can consider un-prefixing because our implementation isn't so far from Webkit's. However, there is no spec nor a guarantee of consensus on details.
Disabling the pseudo-element shouldn't be too much of a pain because our default behaviour uses opacity and should JustWork for [virtually] all cases.
Flags: needinfo?(dbaron)
Flags: needinfo?(bzbarsky)
I'm not sure I have a useful opinion on this.  :(
Flags: needinfo?(bzbarsky)
I'd be hesitant to ship unprefixed without at least making an effort to get it into a CSSWG document.  That probably means either http://dev.w3.org/csswg/css4-pseudo/ or http://dev.w3.org/csswg/css3-ui/ or css4-ui (no document yet).

What did you mean by disabling it for the release?  Backing out the patches and reverting to the :-moz-placeholder pseudo-class (which I think would fix the issue)?  Or making it unusable from content but still usable by the UA style sheet (which I don't think would fix the issue here, but which might avoid future similar problems)?  Or just re-adding a :-moz-placeholder pseudo class temporarily that doesn't do anything (which I think would fix the issue)?  Or some combination of these?
Flags: needinfo?(dbaron)
I will make :-moz-placeholder be parsed but doing nothing.
By "making [::-moz-placeholder] unusable", I meant that ::-moz-placeholder will not be usable by regular content so we will not end up in the same situation. I don't want to have to parse :-moz-placeholder, ::-moz-placeholder and ::placeholder in the future.
(In reply to Mounir Lamouri (:mounir) from comment #30)
> I will make :-moz-placeholder be parsed but doing nothing.

That seems acceptable (barely) as a temporary measure but not a permanent one.

> By "making [::-moz-placeholder] unusable", I meant that ::-moz-placeholder
> will not be usable by regular content so we will not end up in the same
> situation. I don't want to have to parse :-moz-placeholder,
> ::-moz-placeholder and ::placeholder in the future.

That sounds fine.
Attached patch Patch (deleted) — — Splinter Review 
Allow :-moz-placeholder to be parsed but do nothing with it.
Attachment #704638 - Flags: review?(dbaron)
Comment on attachment 704638 [details] [diff] [review]
Patch

I'd prefer if you added a test item to test_selectors.html than added a whole new test.

Otherwise, r=dbaron, though I'd appreciate if bz also had a look at this since we're talking about landing it on beta.  (But feel free to land if the tree opens before bz gets to it.)
Attachment #704638 - Flags: review?(dbaron)
Attachment #704638 - Flags: review?(bzbarsky)
Attachment #704638 - Flags: review+
Comment on attachment 704638 [details] [diff] [review]
Patch

r=me.

Note that with this patch :not(:-moz-placeholder) will match everything.  I think that's fine.
Attachment #704638 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/f8f4c3163cd9
Status: NEW → ASSIGNED
status-firefox21: --- → fixed
Flags: in-testsuite+
Target Milestone: --- → mozilla21
Version: unspecified → Trunk
Comment on attachment 704638 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 737786
User impact if declined: mail.com and gmx mail doesn't work
Testing completed (on m-c, etc.): try + m-i, soon m-c
Risk to taking this patch (and alternatives if risky): This is a bit risky because it is touching the css parser but it shouldn't have any effect on other stuff than :-moz-placeholder or ::-moz-placeholder. Theoretically the regressions should be limited to the pseudo-element behaviour. However, it is heavily tested, such as the css parser.
String or UUID changes made by this patch: none
Attachment #704638 - Flags: approval-mozilla-beta?
Attachment #704638 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/f8f4c3163cd9
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment on attachment 704638 [details] [diff] [review]
Patch

Given the fact that:

1) Outreach has been unsuccessful
2) Other sites may be similarly affected
3) We know of 2 fairly major web properties that are impacted

Let's fix this on all branches as soon as possible and backout at the first sign of regression or sites fixing on their end.
Attachment #704638 - Flags: approval-mozilla-beta?
Attachment #704638 - Flags: approval-mozilla-beta+
Attachment #704638 - Flags: approval-mozilla-aurora?
Attachment #704638 - Flags: approval-mozilla-aurora+
Justification #4 is that this is the lowest risk fix we know of.
I got caught by a change that wasn't applied in mozilla-beta, required a follow-up:
https://hg.mozilla.org/releases/mozilla-beta/rev/e6fd32bde4f8
Saw the issue on FF 19b3.
Verified fixed on FF 19b4 on Win7, Ubuntu 12.04, Mac OS X 10.7.5 using the STR in comment 2 and the testcase in comment 5.
I also took a look over couple other mailing sites (yahoo, gmail, hotmail, aol) for any possible regressions and everything looked ok too.
status-firefox19: fixed → verified
I confirm the fix is Verified on FF 20RC on Windows 7 x64, Mac OS 10.8 and Ubuntu 12.04.
status-firefox20: fixed → verified
Verified fixed on FF 21b1 on Mac OS 10.7 and Windows 7 x64:
build id: (20130401192816)
Status: RESOLVED → VERIFIED
status-firefox21: fixed → verified
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: