Closed Bug 408127 Opened 18 years ago Closed 1 year ago

UI Elements/Events to be instrumented

Categories

(Toolkit Graveyard :: Data Collection/Metrics, defect)

Product:
Toolkit Graveyard
Component:
Data Collection/Metrics
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED INCOMPLETE

People

(Reporter: alex, Assigned: pete)

Details

Attachments

(1 file)

first pass
3.54 KB, patch
janv
: review+
Details | Diff | Splinter Review
Some UI events do not report targets, so they are not logged. This is the tracker bug for known UI interactions that should be trackable. To kick off this bug, I'll add the "quick find" toolbar. This is opened by typing '/' into a browser window. We should be able to capture a UI event to determine if this feature is used.
Clicking the "star" for quick bookmarking should be instrument-able.
Clicking the favicon, which in turn opens up Larry, should be instrument-able.
Assignee: nobody → pete
Status: NEW → ASSIGNED
I think in order to capture these events, we need to add key and click listeners to the UI command collector. Currently the only UI listeners we are are "command" and "TabMove".
(In reply to comment #3) > I think in order to capture these events, we need to add key and click > listeners to the UI command collector. > > Currently the only UI listeners we are are "command" and "TabMove". but shouldn't the actions cause a command? For instance, wouldn't a star click cause a new bookmark command to fire?
I didn't try the star click but entering a "/" to activate the find bar isn't recognized as a command where Apple+F is.
> but shouldn't the actions cause a command? For instance, wouldn't a star click > cause a new bookmark command to fire? If there were commands attached to these controls we could monitor them: <image id="star-button" onclick="PlacesStarButton.onClick(event);"/> A command would have to be registered and accessed somewhere in the call chain for this to be logged w/ our current command observer. If we added other listeners such as "click", then we would need to listen for specific event targets as to narrow down to the clicks we are interested in and not impact performance.
Something like this would work: <image id="star-button" onclick="if (event.button==0) goDoCommand('starbutton_click'); "/> Where we have the command 'starbutton_click' registered.
Alex, I can come up with a test patch to verify this if you wish.
at some point it was capturing star clicks: -1610559488[507150]: Original Target Id: star-button, Original Target Anonid: , Target Id: star-button does this mean the command associated with the star-button was removed? If so, how does it still function?
It currently functions by calling functions directly. Commands are registered identifiers that when called will proxy calls to various functions. I would venture to guess that the newer places code could have replaced older code that used a command to execute this function. Or maybe the star was a button at one point that used an oncommand event handler. We'd have to look at cvs revision history to see what changed.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
I think this bug was inadvertently resolved fixed ...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Larry and the star icon are both popups. Another possibility is to listener for "popupshowing" ... Otherwise we do get notification then the buttons on the popups are clicked.
would popupshowing also allow the '/' to be captured?
(In reply to comment #13) > would popupshowing also allow the '/' to be captured? No, but '/' should be hooked up to a command. I need to look into it and see exactly why it's not being picked up.
Ok, '/' key command is caught by the findbar XBL binding. There is a "keypress" listener listening specifically for these two keyed events - / and '. So these events just run down a javascript call chain so no registered commands are actually called. On possibility is to create a cmd_quickfind and bind the command to the '/' key instead of using the event listener in the findbar binding. This would require changes to browser-sets.inc and findbar.xml. Otherwise we are stuck having to add out own "keypress" event listener and listing for '/' keyed event.
I think the best option is to stick to solutions that do not involve changes to client code. Let move forward on instrumenting the star and larry using popupshowing. Adding key-press listeners seems a bit to evasive, so lets hold on quick-find for now.
Hey Alex, I have popupshowing implemented for Larry and "star". Here is my output: <uielement targetidhash="s4rljiMmGMfpYbwnk/DTzg==" action="popupshowing" window="1" time="1200340322" session="23"/> <uielement targetidhash="wpDgus7KCzTleouB/8b3aA==" action="popupshowing" window="1" time="1200340328" session="23"/> Is this all the data you need? The hashed id's are "identity-popup" and "editBookmarkPanel" respectively ...
Attached patch first pass — — Splinter Review
(In reply to comment #17) > Hey Alex, I have popupshowing implemented for Larry and "star". > > Here is my output: > > <uielement targetidhash="s4rljiMmGMfpYbwnk/DTzg==" action="popupshowing" > window="1" time="1200340322" session="23"/> > <uielement targetidhash="wpDgus7KCzTleouB/8b3aA==" action="popupshowing" > window="1" time="1200340328" session="23"/> > > Is this all the data you need? > > The hashed id's are "identity-popup" and "editBookmarkPanel" respectively ... Perfect! What other popupshowing events will it capture?
All of them, I am just currently listening for Larry and "star" ...
Jan, can you review attachment 297025 [details] [diff] [review] Thanks
Comment on attachment 297025 [details] [diff] [review] first pass r=varga
Attachment #297025 - Flags: review+
Patch checked in. Alex, I'm not sure if you would like to see an patch that listens for '/' find ... It would just be a simple keypress listener that listens only for that event. If not, then I guess we can mark FIXED.
(In reply to comment #23) > Patch checked in. > > Alex, I'm not sure if you would like to see an patch that listens for '/' find > ... > > It would just be a simple keypress listener that listens only for that event. Will it log *anytime* a '/' is typed? Or just under the circumstances that cause the quick find to open? If the later, lets create the patch. Otherwise, I'm gun shy, and let's skip it.
I'm not 100% sure but I think anytime a '/' is typed in the window we will see it. If it is typed in say a urlbar or a form control, I don't think we will see those events ...
Nope, I did a quick test and all key events are logged even in form controls ... So I think the correct solution is for the '/' event to be mapped to an actual command.
(In reply to comment #26) > Nope, I did a quick test and all key events are logged even in form controls > ... Shucks... > So I think the correct solution is for the '/' event to be mapped to an actual > command. OK, lets leave this open and address client changes after Fx3 settles down.
Product: Toolkit → Toolkit Graveyard
Status: REOPENED → RESOLVED
Closed: 18 years ago → 1 year ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: