Status

defect
RESOLVED FIXED
7 years ago
4 months ago

People

(Reporter: mixedpuppy, Assigned: mixedpuppy)

Tracking

unspecified
Firefox 17
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Fx16])

Attachments

(2 attachments, 5 obsolete attachments)

Assignee

Description

7 years ago
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
Assignee

Updated

7 years ago
Depends on: 770365, 755125, 757450, 764265
Whiteboard: [Fx16]
Assignee

Updated

7 years ago
Assignee: nobody → mixedpuppy
Assignee

Comment 1

7 years ago
Posted patch servicewindow.patch (obsolete) — Splinter 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.
Attachment #639512 - Flags: feedback?(gavin.sharp)
Assignee

Comment 2

7 years ago
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.
Assignee

Comment 3

7 years ago
Posted patch servicewindow.patch (obsolete) — Splinter Review
minor bug fix, some renaming of vars to be clear
Attachment #639512 - Attachment is obsolete: true
Attachment #639512 - Flags: feedback?(gavin.sharp)
Assignee

Updated

7 years ago
Duplicate of this bug: 755125
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.
Assignee

Updated

7 years ago
Depends on: 755136
Assignee

Updated

7 years ago
Depends on: 773351
Shane, is this patch ready for a review? Can you set that request if so?
Assignee

Comment 7

7 years ago
(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.
Assignee

Comment 8

7 years ago
Posted patch servicewindow.patch (obsolete) — Splinter Review
initial chat window implementation for feedback
Attachment #639537 - Attachment is obsolete: true
Attachment #642037 - Flags: feedback?(gavin.sharp)
Assignee

Updated

7 years ago
Blocks: 770366
Assignee

Comment 9

7 years ago
Posted patch updated patch (obsolete) — Splinter Review
still needs tests
Attachment #642037 - Attachment is obsolete: true
Attachment #642037 - Flags: feedback?(gavin.sharp)
Attachment #645121 - Flags: review?(gavin.sharp)
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
Attachment #645121 - Flags: review?(gavin.sharp) → review-
Assignee

Comment 11

7 years ago
Posted patch revised patch with tests (obsolete) — Splinter Review
Attachment #645121 - Attachment is obsolete: true
Attachment #645499 - Flags: review?(gavin.sharp)
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.
Attachment #645499 - Attachment is obsolete: true
Attachment #645499 - Flags: review?(gavin.sharp)
Attachment #646024 - Flags: review?(mixedpuppy)
Assignee

Updated

7 years ago
Attachment #646024 - Flags: review?(mixedpuppy) → review+
https://hg.mozilla.org/mozilla-central/rev/9d8cc914ad36
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
OS: Mac OS X → All
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17
Assignee

Updated

7 years ago
Blocks: 774506

Updated

4 months ago
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.