Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Beta 1.2.1 click to dial, limited number of icons #2

Closed
ghost opened this issue May 13, 2015 · 10 comments
Closed

Beta 1.2.1 click to dial, limited number of icons #2

ghost opened this issue May 13, 2015 · 10 comments
Assignees
Labels

Comments

@ghost
Copy link

ghost commented May 13, 2015

G'day,

I'm really impressed with the beta version of the extension but I believe I may have found a limitation.

I've created an internal directory with all of our Active Directory contacts (at the moment there's over 120 entries)...the code for the webpage is as basic as it gets at the moment (just plain html with line breaks) but I'll add on this over time.

Everything appears to work well until you place your cursor over the 90th contact, it's at this point you lose the dial icon to the right of the phone number. Highlighting the number, right clicking using the context menu still works though.

Cheers,
Brad

@tap-chris tap-chris self-assigned this May 13, 2015
@tap-chris
Copy link
Owner

Dear Brad,

Thank you for your report.
I'll generate a similar list, test this issue and let you know as soon as possible.

What happens if you place each phone numer into it's on container?
Like wrap it in a paragraph: <p>number</p>.

Greetings,
Chris

@tap-chris
Copy link
Owner

Dear Brad,

I just created a test case with 1.000 entries. Just number<br />number... which worked fine.
Did you just use \r\n as line breaks? If so then I guess we are hitting a limit of matches in a single string here cause the parser will get all the numbers in a single string. If you use <br /> or a container it should actually work. Can you verify this?
I think the issue you ran into would otherwise be very unlikely to happen but I can still use a different way of matching which allows an unlimited amount of numbers.
Let me know.

Greetings,
Chris

@ghost
Copy link
Author

ghost commented May 13, 2015

G’day,

I just attempted putting them inside containers as suggested ie <p>number</p> and it solved the problem.

I reverted back to the original code and was able to reproduce the same problem, there’s no line breaks or carriage returns just HTML tags providing the formatting.

Here is a sample of the source (numbers modified to protect the innocent):

<br />1AAE Transport<br /><br />Telephone: 131223<br /><br />2AE Baker<br /><br />Telephone: 0292222222<br /><br />3Aaron Lade<br /><br />Telephone: 0288888465<br /><br />4Adept Tapes<br /><br />Telephone: 0247322221<br /><br />5Aevon Polishing<br /><br />Telephone: 0299983931<br /><br />6All Metal Products<br /><br />Telephone: 0299933111<br />

I apologise for the poor code…it was the first step in testing out the syntax of a foreach loop on an array (using PHP to build the page)…the final version will include a form to search for the correct contact so I’d imagine this won’t be an issue when I move forward with the code.

If it helps, I’m currently using Chrome 42.0.2311.135m

Cheers,

Brad Griffiths

@tap-chris
Copy link
Owner

Dear Brad,

No worries about your code, starting simple is always better ;)

The way you did it should actually work since a <br /> is actually also closing the text element.
So in the DOM it should look like HTMLBRElement Text HTMLBRElement HTMLBRElement Text... and so forth. This should parse every Text element and replace the numbers.

I tested it out here on my Chrome 44.0.2398.0 dev-m and it works.
I'll get it tested on a Chrome 42 version and we'll see what happens.

Greetings,
Chris

@ghost
Copy link
Author

ghost commented May 14, 2015

Hi Chris,

Thanks for looking into this, it's a really cool extension and hope this feature set make it to stable status.
I've also confirmed the same symptoms on my laptop at home (Ubuntu) Version 42.0.2311.135 (64-bit)...As an option to help you diagnose the issue, I'm happy to make sure it isn't something silly I've done on my end I'm happy to provide access to the page, there's an IP ACL on the webserver though so I'd need your public IP to give you access to it.

Let me know if there's anything I can do to help.

Cheers,
Brad


From: Christian Volmering [notifications@github.com]
Sent: Thursday, 14 May 2015 9:06 PM
To: tap-chris/cisco-dialer
Cc: Brad Griffiths
Subject: Re: [cisco-dialer] Beta 1.2.1 click to dial, limited number of icons (#2)

Dear Brad,

No worries about your code, starting simple is always better ;)

The way you did it should actually work since a
is actually also closing the text element.
So in the DOM it should look like HTMLBRElement Text HTMLBRElement HTMLBRElement Text... and so forth. This should parse every Text element and replace the numbers.

I tested it out here on my Chrome 44.0.2398.0 dev-m and it works.
I'll get it tested on a Chrome 42 version and we'll see what happens.

Greetings,
Chris


Reply to this email directly or view it on GitHubhttps://github.com//issues/2#issuecomment-102004809.

Notice:This e-mail and any attachments are confidential and are only for the use of the person to whom they are addressed. If you are not the intended recipient please advise the sender by return e-mail and delete the message and any attachments. Any use, interference with, disclosure or copying of this message or any attachments is unauthorised and prohibited. The sender does not warrant that the information is free of a virus or any other defect or error, and any views expressed herein, unless specifically indicated otherwise, are those of the individual sender.

The DH Gibson Group of Companies - http://www.gibsonshopfitters.com.au

P Please consider our shared environment before printing this communication.

@tap-chris
Copy link
Owner

Dear Brad,

You're welcome and thank you very much!
I'm also looking forward to mark the current version stable as soon as it's tested enough.

Looking at the exact page having the issue would actually be of help.
If you send me a mail to christian@volmering.name I'll reply with my current public ips.
Those are changing every 24h thought ;)

Thank you very much for your help!

Greetings,
Chris

@ghost
Copy link
Author

ghost commented May 14, 2015

Thanks, I’ve sent an email including the URL.

Cheers,
Brad Griffiths

@tap-chris tap-chris added bug and removed enhancement labels May 14, 2015
tap-chris added a commit that referenced this issue May 14, 2015
The child count of a node can increase while parsing it if we find
numbers and convert them into dial tooltips. If all the numbers are
childs of one node and we iterate over it then we have to read the
length of the child nodes for each loop otherwise the parser will stop
to early.
@tap-chris
Copy link
Owner

Dear Brad,

Thank you very much for helping me to debug this. I found the bug and referenced it above.

I just released a new beta version which should be live in the Chrome Web Store within the next 60 minutes. Please check if the bug is now handled.

Thanks again and greetings,
Chris

@ghost
Copy link
Author

ghost commented May 14, 2015

I just upgraded and I can confirm the new version has resolved the problem.

I’m always happy to help, if you ever need something tested in a different environment just send me an email.

Cheers,
Brad

@tap-chris
Copy link
Owner

That's nice to hear! 😆

Thank you very much! I'll let you know if I have something new to test out and let me know if you find any other issues or if you need help. I'm also a PHP developer in case you need help with your script 😉

Greetings,
Chris

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant