Closed Bug 1233747 Opened 4 years ago Closed 4 years ago

checkbox label to allow host to focus tab if dialogs shown is odd if page is local file

Categories

(Firefox :: Tabbed Browser, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 46
Tracking Status
firefox44 --- fixed
firefox45 --- fixed
firefox46 --- fixed

People

(Reporter: aryx, Assigned: Gijs)

References

Details

Attachments

(2 files)

Nightly 46.0a1, Aurora 45.0a2 and 44.0 Beta (haven't tested that one)

If you a html file like
<html>
  <body>
    <script>
      var foo = window.setTimeout(function () {alert("here i am");}, 15000);
    </script>
  </body>
</html>
on your local file system and open that file in the browser and select a different tab and then return to the tab after the dialog got opened, it will have a checkbox "Allow dialogs from to take you to their tab".

This feature landed in bug 332195.
Flags: needinfo?(gijskruitbosch+bugs)
Assignee: nobody → gijskruitbosch+bugs
Blocks: 1235709
Status: NEW → ASSIGNED
Flags: needinfo?(gijskruitbosch+bugs)
Comment on attachment 8702845 [details]
MozReview Request: Bug 1233747 and bug 1235709 - deal better with host-less URIs in the checkbox handling for dialogs in non-selected tabs, r?Aryx

https://reviewboard.mozilla.org/r/29195/#review25999
Attachment #8702845 - Flags: review?(aryx.bugmail) → review+
https://hg.mozilla.org/mozilla-central/rev/718d15422bb3
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Comment on attachment 8702845 [details]
MozReview Request: Bug 1233747 and bug 1235709 - deal better with host-less URIs in the checkbox handling for dialogs in non-selected tabs, r?Aryx

Approval Request Comment
[Feature/regressing bug #]: bug 332195
[User impact if declined]: broken label in a tab-modal dialog and/or totally busted modal dialog (see bug 1235709) when using a tab-modal dialog from file:, data: or (specific) about URIs.
[Describe test coverage new/current, TreeHerder]: there is coverage for the dialogs as a whole, but not for these specific cases
[Risks and why]: fairly low - this moves the "determine the host of this URI" code earlier, and puts it in a try catch. The exception from that code was what caused both this and bug 1235709. We then don't bother trying to show the label for data: or about: or file: URIs (and/or any other URLs with no host)
[String/UUID change made/needed]: no (!)
Attachment #8702845 - Flags: approval-mozilla-beta?
Attachment #8702845 - Flags: approval-mozilla-aurora?
Comment on attachment 8702845 [details]
MozReview Request: Bug 1233747 and bug 1235709 - deal better with host-less URIs in the checkbox handling for dialogs in non-selected tabs, r?Aryx

The patch is small, should be safe. Beta44+, Aurora45+
Attachment #8702845 - Flags: approval-mozilla-beta?
Attachment #8702845 - Flags: approval-mozilla-beta+
Attachment #8702845 - Flags: approval-mozilla-aurora?
Attachment #8702845 - Flags: approval-mozilla-aurora+
Depends on: 1236126
Blocks: 1234957
Comment on attachment 8702845 [details]
MozReview Request: Bug 1233747 and bug 1235709 - deal better with host-less URIs in the checkbox handling for dialogs in non-selected tabs, r?Aryx

We should uplift this together with the one-line fix from bug 1236126, once that gets review and lands.
Attachment #8702845 - Flags: approval-mozilla-beta+
Attachment #8702845 - Flags: approval-mozilla-aurora+
Comment on attachment 8703571 [details] [diff] [review]
Patch for uplift (includes this + bug 1236126)

Re-requesting approval for this + bug 1236126. Sorry for the churn.

Approval Request Comment
[Feature/regressing bug #]: bug 332195
[User impact if declined]: broken label in a tab-modal dialog and/or totally busted modal dialog (see bug 1235709) when using a tab-modal dialog from file:, data: or (specific) about URIs.
[Describe test coverage new/current, TreeHerder]: there is coverage for the dialogs as a whole, but not for these specific cases
[Risks and why]: fairly low - this moves the "determine the host of this URI" code earlier, and puts it in a try catch. The exception from that code was what caused both this and bug 1235709. We then don't bother trying to show the checkbox and its label for data: or about: or file: URIs (and/or any other URLs with no host)
[String/UUID change made/needed]: no (!)
Attachment #8703571 - Attachment description: Patch for uplift (includes this + ) → Patch for uplift (includes this + bug 1236126)
Attachment #8703571 - Flags: review+
Attachment #8703571 - Flags: approval-mozilla-beta?
Attachment #8703571 - Flags: approval-mozilla-aurora?
Attachment #8703571 - Flags: approval-mozilla-beta?
Attachment #8703571 - Flags: approval-mozilla-beta+
Attachment #8703571 - Flags: approval-mozilla-aurora?
Attachment #8703571 - Flags: approval-mozilla-aurora+
[bugday-20160323]

Status: RESOLVED,FIXED -> UNVERIFIED

Comments:
STR: 
Test partially successful.

Component: 
Name			Firefox
Version			46.0b9
Build ID		20160322075646
Update Channel	        beta
User Agent		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS			Windows 7 SP1 x86_64

Expected Results: 
return to the tab after the dialog got opened, it will have a checkbox "Allow dialogs from to take you to their tab".


Actual Results: 
No dialog is appearing except "here I am"
You need to log in before you can comment on or make changes to this bug.