Closed
Bug 914447
Opened 11 years ago
Closed 11 years ago
Defect - Monocles not always appearing around selected URL in Navigation App Bar
Categories
(Firefox for Metro Graveyard :: App Bar, defect, P2)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 28
People
(Reporter: kjozwiak, Assigned: jimm)
References
Details
(Whiteboard: [beta28] feature=defect u=metro_firefox_user c=content_features p=1)
Attachments
(2 files)
(deleted),
image/jpeg
|
Details | |
(deleted),
patch
|
mbrubeck
:
review+
|
Details | Diff | Splinter Review |
When tapping on the Navigation App Bar to select the URL, sometimes the monocles will not appear at both ends of the selected URL.
- Attached a screenshot to illustrate the issue (notice the monocles missing)
Steps to reproduce the issue:
1) Open Firefox Metro
2) Go to wikipedia.org
3) Slide in the Navigation App Bar from the bottom and tap on the URL (entire URL should be selected and a monocle on both ends should appear)
4)Slide in the Windows Charm from the right side of the screen (the monocles should disappear)
5) Tap on the website to dismiss the Windows Charm and then re-tap the URL in the Navigation App Bar (monocles should re-appear)
6) Go through Step #4 & #5 a few times (sometimes it takes 10-15 times) and you'll notice that sometimes the monocles will not appear
Current Behavior:
- Monocles not always appearing when tapping on the URL under the Navigation App Bar
Expected Behavior:
- Monocles should be appearing every single time when tapping on the URL under the Navigation App Bar
Updated•11 years ago
|
Whiteboard: feature=defect u=metro_firefox_user c=content_features p=0 → [preview-triage] feature=defect u=metro_firefox_user c=content_features p=0
Updated•11 years ago
|
Blocks: MetroPreviewRelease
Summary: Defect - Monocles not always appearing around selected URL in Navigation App Bar → [MP] Defect - Monocles not always appearing around selected URL in Navigation App Bar
Whiteboard: [preview-triage] feature=defect u=metro_firefox_user c=content_features p=0 → [preview] feature=defect u=metro_firefox_user c=content_features p=0
Updated•11 years ago
|
No longer blocks: MetroPreviewRelease
Summary: [MP] Defect - Monocles not always appearing around selected URL in Navigation App Bar → Defect - Monocles not always appearing around selected URL in Navigation App Bar
Whiteboard: [preview] feature=defect u=metro_firefox_user c=content_features p=0 → feature=defect u=metro_firefox_user c=content_features p=0
Updated•11 years ago
|
Whiteboard: feature=defect u=metro_firefox_user c=content_features p=0 → [release28] feature=defect u=metro_firefox_user c=content_features p=0
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → jmathies
Blocks: metrov1it20
Whiteboard: [release28] feature=defect u=metro_firefox_user c=content_features p=0 → [beta28] feature=defect u=metro_firefox_user c=content_features p=1
Assignee | ||
Comment 1•11 years ago
|
||
(In reply to Kamil Jozwiak [:kjozwiak] from comment #0)
> 1) Open Firefox Metro
> 2) Go to wikipedia.org
> 3) Slide in the Navigation App Bar from the bottom and tap on the URL
> (entire URL should be selected and a monocle on both ends should appear)
> 4)Slide in the Windows Charm from the right side of the screen (the monocles
> should disappear)
> 5) Tap on the website to dismiss the Windows Charm and then re-tap the URL
> in the Navigation App Bar (monocles should re-appear)
> 6) Go through Step #4 & #5 a few times (sometimes it takes 10-15 times) and
> you'll notice that sometimes the monocles will not appear
Excellent str, thanks for this.
Updated•11 years ago
|
Assignee | ||
Comment 2•11 years ago
|
||
This was caused by clicks that fell within the bounds of the urlbar but outside the bounds of internal text input. When we sent this over, the selection handler didn't find an editable at the event coordinates.
Note the 10 px offset feels a little cheesy, maybe there's a way to dig down and get the actual text input box object? Regardless this still works reliably.
Also added a little cleanup and commenting that seemed lacking when I first started debugging this.
Attachment #8343089 -
Flags: review?(mbrubeck)
Assignee | ||
Comment 3•11 years ago
|
||
Comment on attachment 8343089 [details] [diff] [review]
patch v.1
Review of attachment 8343089 [details] [diff] [review]:
-----------------------------------------------------------------
::: browser/metro/base/content/bindings/bindings.xml
@@ +199,4 @@
> SelectionHelperUI.attachEditSession(ChromeSelectionHandler,
> event.clientX, event.clientY);
> } else {
> + dump("click: sending observer\n");
removed these dumps from my local patch.
Comment 4•11 years ago
|
||
Comment on attachment 8343089 [details] [diff] [review]
patch v.1
Review of attachment 8343089 [details] [diff] [review]:
-----------------------------------------------------------------
r=mbrubeck with some minor changes:
::: browser/metro/base/content/browser.xul
@@ +246,5 @@
> autocompletepopup="urlbar-autocomplete"
> completeselectedindex="true"
> placeholder="&urlbar.emptytext;"
> tabscrolling="true"
> + onclick="SelectionHelperUI.urlbarTextboxClick(this);"/>
Try passing (this.inputField) instead of (this) -- that should be a reference to the internal <html:input> element, which will maybe eliminate the need for the 10px offset?
::: browser/metro/base/content/helperui/SelectionHelperUI.js
@@ +532,5 @@
> + }
> +
> + // Attempt to enable selection if there's text in the control.
> + this.attachEditSession(ChromeSelectionHandler,
> + aEdit.boxObject.screenX + 10, aEdit.boxObject.screenY + 10);
If we keep the 10px offset, this needs a better comment explaining it.
Attachment #8343089 -
Flags: review?(mbrubeck) → review+
Assignee | ||
Comment 5•11 years ago
|
||
> Try passing (this.inputField) instead of (this) -- that should be a
That's what I was looking for, thanks. There wasn't a boxObject on this element so I used data in getBoundingClientRect(), which worked.
Assignee | ||
Comment 6•11 years ago
|
||
Comment 7•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 28
Updated•10 years ago
|
OS: Windows 8 Metro → Windows 8.1
You need to log in
before you can comment on or make changes to this bug.
Description
•