VoIPGRID / chrome-extension

VoIPGRID Chrome Extension
MIT License
7 stars 9 forks source link

Phone numbers not recognized in gmail #3

Closed cpoppema closed 10 years ago

cpoppema commented 11 years ago

For example: install this plugin and browse to https://mail.google.com/mail/u/0/?shva=1#contacts. In case you have phone numbers for your contacts, the plugin does not detect these.

Also applicable to other google services like google search (maybe more)

wesleylancel commented 11 years ago

Doesn't have anything to do with Google I think, it only checks for phone numbers when the DOM has finished loading. Phone numbers (like in Gmail) that are added afterwards are not recognized.

cpoppema commented 11 years ago

You're correct, but it serves as a nice test case :)

wesleylancel commented 11 years ago

Any of you looking into this? I'm willing to take a look at this somewhere this week. Interesting use case for MutationObserver :)

cpoppema commented 11 years ago

We're not looking into this right now, because there is always the option to select the phone number as text and call it via the context menu. Personally I have no experience with MutationObserver, but what I had in mind so far: we need to listen to the native event DOMSubtreeModified.

window.addEventListener('DOMSubtreeModified', function (event) {
    if(event.constructor.name === 'MutationEvent') {
        // modified DOM elements can be found in either of these I believe, 
        // event.target being the parent element of event.target.children
        event.target;
        event.target.children;  
    }
});

However, doing this requires the search for phone numbers to be more intelligent because there is the possibility phone numbers found by the regex on the changed target have been found previously and already have an phone icon. Nevertheless I haven't tested if this is the way to go, but it's a start. Looking forward to your input on this!

wesleylancel commented 11 years ago

DOMSubtreeModified is part of Mutation Events (https://developer.mozilla.org/en-US/docs/Web/Guide/DOM/Events/Mutation_events) which has been deprecated in favor of the new MutationObserver.

I'll see if there's some smart way to check modified items, but as a beginning all added items could be parsed by the regex, this would probably suffice for most cases.

Ivbul commented 10 years ago

Fixed, please test.

wesleylancel commented 10 years ago

@Ivbul: What commit is the code in? I'm willing to test after a I know which code to checkout :)

Ivbul commented 10 years ago

Sorry, I've forgot to commit the code. I'll do it later today.

Ivbul commented 10 years ago

Commited.

wesleylancel commented 10 years ago

Hi @Ivbul,

I just checked out your code and tested a bit, it doesn't seem to pickup any phone numbers in the Google Contacts sections yet. Any phonenumbers that I inject into the DOM using some Javascript aren't picked up either.

wesleylancel commented 10 years ago

@Ivbul:

I added a pull request here (https://github.com/VoIPGRID/chromeextension/pull/15) that fixes part of the problem. Mutations are now detected and when they are not phone numbers they are not replaced (this throws an exception otherwise). However, not all phone numbers are recognized yet.

Ivbul commented 10 years ago

I can’t check it without Holland contacts in the contact list. Could you please send me the contact list in CSV format for testing?

cpoppema commented 10 years ago

Sure, I have attached a file in Basecamp.

Ivbul commented 10 years ago

I fixed one bug with mutation observer. In observer configuration I added some fixes.

There was a bug: If we go to the contact page. Page was parse and we can see phone icons. After that clicking on edit contact and then click at the back batton. After that contacts page has no icons phone.

This bug has been fixed.

And also we have a some cutup bug. (see at the picture). image

We can fix it if replace phone icon to the start of phone number. It related with element width and overflow.

After changes, we get: image

Similar changes should be in Firefox plugin, I think.

cpoppema commented 10 years ago

Thanks and thank you for spotting the other issue, I will look into this.

cpoppema commented 10 years ago

After discussing this, we decided to not make any changes here. It is fine to keep the icon behind the phonenumber: this works better in most cases. In the case of GMail contacts, users can still use the right mouse button on the phonenumber and select 'Bel geselecteerde nummer' (call selected number) to start an outgoing call (even with the dots in place).

In the future we might be looking at removing styles on mouseover like fixed width or text-overflow ellipsis to expose the phone icon, but not right now.