Google CalDAV OAuth2 authentication dialog not shown in SeaMonkey

RESOLVED FIXED in seamonkey2.28

Status

defect
RESOLVED FIXED
6 years ago
9 months ago

People

(Reporter: ssitter, Assigned: Fallen)

Tracking

SeaMonkey 2.22 Branch
seamonkey2.28

SeaMonkey Tracking Flags

(seamonkey2.25 affected, seamonkey2.26 fixed, seamonkey2.27 fixed, seamonkey2.28 fixed)

Details

Attachments

(1 attachment)

Lightning 2.7a1 + SeaMonkey 2.22a1

STR:
1. connect to Google Calendar using the new CalDAV endpoint 
   https://apidata.googleusercontent.com/caldav/v2/{calid}/events

Actual:
Nothing happens after closing the new calendar wizard. Google CalDAV OAuth2 authentication dialog is not shown. No error is shown in Console.
Hey Stefan, sorry I didn't see this bug a month ago.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 919018
I'm note sure this is a duplicated of Bug 919018. 

In Bug 919018 CalDAV is reported broken because the http.jsm module is missing in SeaMonkey 2.21. In this report I used SeaMonkey 2.22 that was build several weeks after Bug 884319	
was fixed and that should contain the http.jsm module. In addition I did not saw any error messages as written in comment #0.

More testing and feedback from SeaMonkey users would be nice.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
This is still not working in SeaMonkey 2.22. The OAuth window doesn't appear.
Just to be sure, is this specifically an issue with 2.22, or is it an ongoing issue with the latest releases and nightly Seamonkey builds?
Priority: -- → P2
Ongoing
Timestamp: 4/1/14, 11:26:07 PM
Error: Error: Permission denied for <moz-safe-about:neterror?e=fileNotFound&u=chrome%3A//messenger/content/browserRequest.xul&c=UTF-8&f=regular&d=The%20file%20chrome%3A//messenger/content/browserRequest.xul%20cannot%20be%20found.%20Please%20check%20the%20location%20and%20try%20again.> to create wrapper for object of class UnnamedClass
Assignee: nobody → philipp
Status: REOPENED → ASSIGNED
Component: Lightning: SeaMonkey Integration → General
OS: Windows 7 → All
Priority: P2 → --
Product: Calendar → SeaMonkey
Hardware: x86_64 → All
Version: Lightning 2.7 → SeaMonkey 2.22 Branch
Seamonkey is missing browserRequest.* and its artwork. I've tested this by manually injecting the files into a Seamonkey nightly and I must admit this patch is totally untested. Nevertheless, I hope I've copied all the files into the right locations.

I'd appreciate if you could give this patch a test run.
Attachment #8400232 - Flags: review?(philip.chee)
Attachment #8400232 - Flags: ui-review?(neil)
Comment on attachment 8400232 [details] [diff] [review]
Fix - v1 [check-in comment 13][c-a,c-b check-in comment 16]

>     const wpl_security_bits = wpl.STATE_IS_SECURE |
>                               wpl.STATE_IS_BROKEN |
>                               wpl.STATE_IS_INSECURE |
>                               wpl.STATE_SECURE_HIGH |
>                               wpl.STATE_SECURE_MED |
>                               wpl.STATE_SECURE_LOW;
>     var browser = document.getElementById("requestFrame");
>     var level;

>     switch (aState & wpl_security_bits) {
>       case wpl.STATE_IS_SECURE | wpl.STATE_SECURE_HIGH:
>         level = "high";
>         break;
>       case wpl.STATE_IS_SECURE | wpl.STATE_SECURE_MED:
>       case wpl.STATE_IS_SECURE | wpl.STATE_SECURE_LOW:
>         level = "low";
>         break;
>       case wpl.STATE_IS_BROKEN:
>         level = "broken";
>         break;

> #security-button[level="high"],
> #security-button[level="low"] {
>   background-image: url("chrome://messenger/skin/icons/secure.png");
> }

Your levels high and low are identical in the UI. You should simplify this like (for example) https://hg.mozilla.org/comm-central/rev/24ad7887d181#l2.7
See Bug 810671.

>     this.securityButton.setAttribute("tooltiptext",
>                                      browser.securityUI.tooltipText);
tooltipText property has been removed. See Bug 832848.

> function loadRequestedUrl()
> {
>   let request = window.arguments[0].wrappedJSObject;
>   document.getElementById("headerMessage").textContent = request.promptText;

I think that the promptText should go to the window title, because several lines further down you set the headerMessage to the url.

>   let account = request.account;
>   if (request.iconURI != "")

if (request.iconURI)

>     document.getElementById("headerImage").src = request.iconURI;
"headerImage" doesn't exist. Looking at the chat version of this file I think you need "security-button". Actually you don't since it's so overloaded.

>   var url = request.url;
>   if (url != "") {
if (url)

>     <image id="security-button" src="chrome://branding/content/icon32.png"/>
Don't hard code paths to theme images. Theme authors really really hate this. You already have browserRequest.css so should move this into CSS.

> #security-button {
  list-style-image: url("chrome://branding/content/icon32.png")
> }

> #security-button[level="high"],
> #security-button[level="low"] {
>   background-image: url("chrome://messenger/skin/icons/secure.png");
> }
This is XUL so please use list-style-image. With list-style-image you can get rid of background-repeat and background-position.

> #security-button[level="broken"] {
>   background-image: url("chrome://messenger/skin/icons/insecure.png");
> }

> #security-button[loading="true"] {
>   background-image: url("chrome://messenger/skin/icons/loading.png");
>   background-position: 4px 2px;
> }

So you start off with a 32x32px icon. Then switch to a 16x16px spinner and then your end state is a 22x22px secure/insecure image. Actually the latter is 18x18px with a 2px margin built in. A really horrendous bit of coding here...

>     <image id="security-button" src="chrome://branding/content/icon32.png"/>
Also because the image src is used. At the end point of the procedure I see a SeaMonkey 32px icon on top of the 22px secure icon. I don't understand how the Thunderbird reviewers missed this.

> +  skin/classic/messenger/icons/loading@2x.png                           (mac/messenger/icons/loading@2x.png)
I think if we want to support retina displays we should have 2x for all of loading/secure/insecure/broken or none at all.

>   background-image: url("chrome://messenger/skin/icons/secure.png");
>   background-image: url("chrome://messenger/skin/icons/insecure.png");

> +  skin/classic/messenger/icons/insecure.png                             (messenger/icons/insecure.png)
> +  skin/classic/messenger/icons/loading.png                              (messenger/icons/loading.png) 

I see some 16x16px images that we could use instead in /chat/themes/ available-16.png and away-16.png.

> /*
> #header {
>   overflow: hidden;
>   padding: 5px;
>   border-bottom: 1px solid black;
>   font-weight: bold;
>   font-size: 1.2em;
> }
> */
This whole comment should be removed.

> diff --git a/mail/themes/linux/mail/browserRequest.css b/suite/themes/classic/messenger/browserRequest.css
> copy from mail/themes/linux/mail/browserRequest.css
> copy to suite/themes/classic/messenger/browserRequest.css
> diff --git a/mail/themes/linux/mail/icons/insecure.png b/suite/themes/classic/messenger/icons/insecure.png
> copy from mail/themes/linux/mail/icons/insecure.png
> copy to suite/themes/classic/messenger/icons/insecure.png 
For SeaMonkey Classic, please copy from the winstripe versions instead.
Attachment #8400232 - Flags: review?(philip.chee)
I understand that the files copied from Thunderbird may not be in good shape, but I really just created this patch as a quick copy from TB to SM so that users are happy. I didn't expect this to be a full cleanup, which would mean clean up in TB, SM and IB together. Is there any chance you'd r+ the patch with just the Seamonkey related bits fixed, i.e copying the icons from winstripe instead?
Comment on attachment 8400232 [details] [diff] [review]
Fix - v1 [check-in comment 13][c-a,c-b check-in comment 16]

(In reply to Philipp Kewisch [:Fallen] from comment #9)
> I understand that the files copied from Thunderbird may not be in good
> shape, but I really just created this patch as a quick copy from TB to SM so
> that users are happy. I didn't expect this to be a full cleanup, which would
> mean clean up in TB, SM and IB together.

Sorry for getting carried away. r=me for landing in suite on condition you file a followup bug to fix the issues detailed Comment 8. I'll probably work on getting it into shape and then moved to shared /mailnews/

> patch with just the Seamonkey related bits fixed, i.e copying the icons from
> winstripe instead?
Sure.
Attachment #8400232 - Flags: review+
> Sorry for getting carried away. r=me for landing in suite on condition you
> file a followup bug to fix the issues detailed Comment 8. I'll probably work
> on getting it into shape and then moved to shared /mailnews/
I guess I can reuse this bug for follow-up patches after you get ui-r from Neil().
Comment on attachment 8400232 [details] [diff] [review]
Fix - v1 [check-in comment 13][c-a,c-b check-in comment 16]

Well, that was awful.

First, I accidentally created the calendar while I was offline. Nothing happened. I couldn't find out how to access the calendar after creating it, nor could I delete the calendar, so I resorted to deleting all calendar prefs from prefs.js just so that I could recreate the calendar so as to trigger the authentication.

The window that was opened wasn't sure whether it was supposed to be a dialog. It had the title bar height of a regular window, but the lack of window features of a dialog. It was also always on top of other windows from the same application, so I couldn't easily switch to, say, DOM Inspector. It also had no title.

The window included something that seemed to want to resemble a URL bar, but it had a strange icon and was poorly themed. DOM Inspector showed that the URL wanted to wrap although its overflow had been hidden not once, but twice.
Attachment #8400232 - Flags: ui-review?(neil)
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/e07e871681ab
Target Milestone: --- → seamonkey2.28
Attachment #8400232 - Attachment description: Fix - v1 → Fix - v1 [check-in comment 13]
Comment on attachment 8400232 [details] [diff] [review]
Fix - v1 [check-in comment 13][c-a,c-b check-in comment 16]

[Approval Request Comment]
Regression caused by (bug #): bug 853236 - Provide access to Google CalDAV v2 API
User impact if declined: Lightning users won't be able to sync with Google calendar.
Testing completed (on m-c, etc.): comm-central
Risk to taking this patch (and alternatives if risky): low to none.
String changes made by this patch: None.
Attachment #8400232 - Flags: approval-comm-beta?
Attachment #8400232 - Flags: approval-comm-aurora?
Comment on attachment 8400232 [details] [diff] [review]
Fix - v1 [check-in comment 13][c-a,c-b check-in comment 16]

a=me
Attachment #8400232 - Flags: approval-comm-beta?
Attachment #8400232 - Flags: approval-comm-beta+
Attachment #8400232 - Flags: approval-comm-aurora?
Attachment #8400232 - Flags: approval-comm-aurora+
Attachment #8400232 - Attachment description: Fix - v1 [check-in comment 13] → Fix - v1 [check-in comment 13][c-a,c-b check-in comment 16]
Whiteboard: [leave open]
Yargh. I think I landed the patch on the wrong branch in comm-beta. Running graft now.
comm-beta: graft to default/tip
http://hg.mozilla.org/releases/comm-beta/rev/43d59f510a7e
http://hg.mozilla.org/releases/comm-beta/graph/43d59f510a7e
Status: ASSIGNED → RESOLVED
Closed: 6 years ago5 years ago
Resolution: --- → FIXED
Whiteboard: [leave open]
Ah, hg copy, that would explain why the tooltipText code still exists after it was removed in Gecko 24...
You need to log in before you can comment on or make changes to this bug.