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.

Sign up for GitHub

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

Jump to bottom

Unchecking the "Open in new window" checkbox does nothing in Firefox #939

Closed
lydell opened this issue Jan 20, 2016 · 1 comment
Closed

Unchecking the "Open in new window" checkbox does nothing in Firefox #939

lydell opened this issue Jan 20, 2016 · 1 comment
Labels
browser-firefox bug easy first bug

Comments

@lydell
Copy link

lydell commented Jan 20, 2016

Steps to reproduce in Firefox:

  1. Hover over a link with the target="_blank" attribute (either add it in the
    HTML source code, or add it by using MediumEditor) and click the URL popup
    that appears to edit the link. The link I used is very simple: It contains
    just one single, unformatted word.
  2. In the URL edit form that opens, uncheck the "Open in new window" (make sure
    that it is checked).
  3. Click the checkmark in the URL edit form to confirm the changes.

In Firefox (Developer Edition 45.0a2 on Ubuntu 15.04 in my case), the
target="_blank" attribute is still there. (I inspected the link with the
devtools to see it. You can also re-open the URL edit form and see that the
"Open link in new window" checkbox has been "re-checked"). In Chromium,
target="_blank" is removed as expected when unchecking the checkbox.

When running the debugger, I found that this line was executed:

medium-editor/src/js/core.js

Line 1043 in f7c522a

this.options.ownerDocument.execCommand('createLink', false, opts.url);

The code seems to rely on that running document.execCommand('createLink', false, url) returns a fresh link without a target attribute. That's the case
in Chromium, but not in Firefox.

I made a reduced test case that only uses that line of code to demonstrate what
happens.

  1. Open the following page:

    <!doctype html>
<title>contenteditable</title>
<div contenteditable="true"><a href="/foo" target="_blank">test</a></div>

  2. Select the entire link with the mouse (the word "test").

  3. Run the following code in the console while the link is selected:

    document.execCommand('createLink', false, '/bar')

  4. Inspect the link with the devtools. Notice how the href attribute has
    changed from /foo to /bar, but the target="_blank" attribute remained.
    In Firefox, that is. When doing the same in Chromium, the target attribute
    vanishes (or, perhaps, isn't preserved, depending on how you look at it).

I suggest explicitly removing the target attribute to fix this problem.
@nmielnik
Copy link
Member

Thanks for pointing this out @lydell , if you have a chance to submit a fix for this we'd be glad to accept it, especially since you've been able to narrow down the code and describe an excellent test scenario.

Otherwise, hopefully I'll have some time to look into it or someone else might take a crack at fixing it 😄

rishijain pushed a commit that referenced this issue Feb 11, 2016
fix for #939 🔨
ded26d5
@rishijain rishijain mentioned this issue Feb 11, 2016
Merged
nmielnik added a commit that referenced this issue Feb 12, 2016
@nmielnik
Merge pull request #967 from yabwe/fix939
5087f4b 
fix for #939 🔨
@j0k3r j0k3r closed this as completed Feb 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
browser-firefox bug easy first bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@j0k3r @lydell @nmielnik