Last Comment Bug 770695 - land v1 serviceWindow
: land v1 serviceWindow
Product: Firefox
Classification: Client Software
Component: SocialAPI (show other bugs)
: unspecified
: All All
-- normal (vote)
: Firefox 17
Assigned To: Shane Caraveo (:mixedpuppy)
: Shane Caraveo (:mixedpuppy)
: 755125 (view as bug list)
Depends on: 755125 755136 757450 764265 770365 773351
Blocks: 770366 774506
  Show dependency treegraph
Reported: 2012-07-03 15:14 PDT by Shane Caraveo (:mixedpuppy)
Modified: 2013-12-27 14:25 PST (History)
3 users (show) in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

servicewindow.patch (4.68 KB, patch)
2012-07-05 16:38 PDT, Shane Caraveo (:mixedpuppy)
no flags Details | Diff | Splinter Review
servicewindow.patch (4.96 KB, patch)
2012-07-05 17:49 PDT, Shane Caraveo (:mixedpuppy)
no flags Details | Diff | Splinter Review
Mockup: Simplified chat window on OSX and Windows (484.74 KB, image/png)
2012-07-09 21:00 PDT, Jennifer Morrow [:Boriss] (UX)
no flags Details
servicewindow.patch (8.00 KB, patch)
2012-07-13 14:05 PDT, Shane Caraveo (:mixedpuppy)
no flags Details | Diff | Splinter Review
updated patch (8.75 KB, patch)
2012-07-23 17:07 PDT, Shane Caraveo (:mixedpuppy) review-
Details | Diff | Splinter Review
revised patch with tests (18.49 KB, patch)
2012-07-24 14:31 PDT, Shane Caraveo (:mixedpuppy)
no flags Details | Diff | Splinter Review
slightly modified patch, with additional test (20.90 KB, patch)
2012-07-25 21:58 PDT, :Gavin Sharp [email:]
mixedpuppy: review+
Details | Diff | Splinter Review

Description User image Shane Caraveo (:mixedpuppy) 2012-07-03 15:14:49 PDT
Initial functionality for the chat window is a chromeless window with a xul:browser that loads content from a provider.

- we ensure the loaded content is over ssl with a valid cert required
- no ssl indicators will be used in the window
- there is an api, implemented in Provider.jsm, available to social iframes allowing the window to be opened from *visible* content
Comment 1 User image Shane Caraveo (:mixedpuppy) 2012-07-05 16:38:57 PDT
Created attachment 639512 [details] [diff] [review]

An implementation for the service window (chat) that doesn't require all the xul/js/module stuff we've been using.  Much simpler, and potentially throw away when we redo chat for 17.

Tests will require the provider class landed, so waiting on that, looking for early review and feedback.
Comment 2 User image Shane Caraveo (:mixedpuppy) 2012-07-05 16:41:16 PDT
another note on this patch, getWebProgressListener is exported since it can be used by the sidebar or other content area's to ensure the content area remains same-origin to the provider.
Comment 3 User image Shane Caraveo (:mixedpuppy) 2012-07-05 17:49:52 PDT
Created attachment 639537 [details] [diff] [review]

minor bug fix, some renaming of vars to be clear
Comment 4 User image Shane Caraveo (:mixedpuppy) 2012-07-09 13:45:08 PDT
*** Bug 755125 has been marked as a duplicate of this bug. ***
Comment 5 User image Jennifer Morrow [:Boriss] (UX) 2012-07-09 21:00:25 PDT
Created attachment 640493 [details]
Mockup: Simplified chat window on OSX and Windows

Drawing our own serviceWindow gives us the control over the widgets and interface of chat windows that we've previously lacked, correct?

While the chat windows won't be able to minimize to the browser window in v1, making their chrome more minimal and more integrated with Firefox's design will go a long way towards their usability.
Comment 6 User image Asa Dotzler [:asa] 2012-07-12 16:49:43 PDT
Shane, is this patch ready for a review? Can you set that request if so?
Comment 7 User image Shane Caraveo (:mixedpuppy) 2012-07-12 17:12:28 PDT
(In reply to Asa Dotzler [:asa] from comment #6)
> Shane, is this patch ready for a review? Can you set that request if so?

There is a line of bugs with patches for review prior to this one.  I am currently reworking this patch on top of those.
Comment 8 User image Shane Caraveo (:mixedpuppy) 2012-07-13 14:05:31 PDT
Created attachment 642037 [details] [diff] [review]

initial chat window implementation for feedback
Comment 9 User image Shane Caraveo (:mixedpuppy) 2012-07-23 17:07:23 PDT
Created attachment 645121 [details] [diff] [review]
updated patch

still needs tests
Comment 10 User image :Gavin Sharp [email:] 2012-07-24 08:19:13 PDT
Comment on attachment 645121 [details] [diff] [review]
updated patch

Here are a couple of comments:
- no need to put openServiceWindow in its own module, just put it in mozSocialAPI directly.
- let's get rid of the webprogresslistener for now (per in-person discussion)
- rather than newURI(aTargetWindow.location.href, ...), you can use aTargetWindow.document.documentURIObject
- need tests
Comment 11 User image Shane Caraveo (:mixedpuppy) 2012-07-24 14:31:59 PDT
Created attachment 645499 [details] [diff] [review]
revised patch with tests
Comment 12 User image :Gavin Sharp [email:] 2012-07-25 21:58:54 PDT
Created attachment 646024 [details] [diff] [review]
slightly modified patch, with additional test

Same patch, just made a few tweaks, and added a test for the double-window case. I was a bit afraid about the use of getWindowByName, so I added a prefix to the name, and specified the "aCurrentWindow" argument to getWindowByName.

I also renamed runTests to runSocialTests when moving it to head.js, because it otherwise conflicts with existing "runTests" in other tests.
Comment 13 User image :Gavin Sharp [email:] 2012-07-26 09:19:48 PDT

Note You need to log in before you can comment on or make changes to this bug.